mailarchive of the pengutronix oss-tools mailing list
 help / color / mirror / Atom feed
* [OSS-Tools] [PATCH platsch V3 1/4] platsch: constify draw_buffer
@ 2024-06-13  7:07 LI Qingwu
  2024-06-13  7:07 ` [OSS-Tools] [PATCH platsch V3 2/4] convert to meson build LI Qingwu
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: LI Qingwu @ 2024-06-13  7:07 UTC (permalink / raw)
  To: Qing-wu.Li, oss-tools, m.felsch

From: Marco Felsch <m.felsch@pengutronix.de>

Make the dir and base const.

Upstream-Status: Pending

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 platsch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/platsch.c b/platsch.c
index 535b589..1aaa8d5 100644
--- a/platsch.c
+++ b/platsch.c
@@ -111,7 +111,7 @@ struct modeset_dev {
 	uint32_t crtc_id;
 };
 
-void draw_buffer(struct modeset_dev *dev, char *dir, char *base)
+void draw_buffer(struct modeset_dev *dev, const char *dir, const char *base)
 {
 	int fd_src;
 	char filename[128];
-- 
2.34.1




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

* [OSS-Tools] [PATCH platsch V3 2/4] convert to meson build
  2024-06-13  7:07 [OSS-Tools] [PATCH platsch V3 1/4] platsch: constify draw_buffer LI Qingwu
@ 2024-06-13  7:07 ` LI Qingwu
  2024-06-13  8:44   ` Michael Tretter
  2024-06-14  5:52   ` Ulrich Ölmann
  2024-06-13  7:07 ` [OSS-Tools] [PATCH platsch V3 3/4] platsch: split into platsch and libplatsch LI Qingwu
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 9+ messages in thread
From: LI Qingwu @ 2024-06-13  7:07 UTC (permalink / raw)
  To: Qing-wu.Li, oss-tools, m.felsch

Convert to meson build and update the README.rst

Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
---
 Makefile.am  | 23 -----------------------
 README.rst   |  8 ++++++++
 configure.ac | 13 -------------
 meson.build  | 15 +++++++++++++++
 4 files changed, 23 insertions(+), 36 deletions(-)
 delete mode 100644 Makefile.am
 delete mode 100644 configure.ac
 create mode 100644 meson.build

diff --git a/Makefile.am b/Makefile.am
deleted file mode 100644
index d149ae0..0000000
--- a/Makefile.am
+++ /dev/null
@@ -1,23 +0,0 @@
-EXTRA_DIST = README.rst LICENSE
-
-sbin_PROGRAMS = platsch
-
-platsch_SOURCES = platsch.c
-platsch_CFLAGS = $(LIBDRM_CFLAGS)
-platsch_LDADD = $(LIBDRM_LIBS)
-
-CLEANFILES = \
-        $(DIST_ARCHIVES)
-
-DISTCLEAN = \
-        config.log \
-        config.status \
-        Makefile
-
-MAINTAINERCLEANFILES = \
-	aclocal.m4 \
-	configure \
-	depcomp \
-	install-sh \
-	Makefile.in \
-	missing
diff --git a/README.rst b/README.rst
index e318120..f1c0812 100644
--- a/README.rst
+++ b/README.rst
@@ -141,3 +141,11 @@ By adding a Signed-off-by line (e.g. using ``git commit -s``) saying::
 
 (using your real name and e-mail address), you state that your contributions
 are in line with the DCO.
+
+Compiling Instructions
+----------------------------
+
+.. code-block:: shell
+
+    meson setup build
+    meson compile -C build
diff --git a/configure.ac b/configure.ac
deleted file mode 100644
index 18878db..0000000
--- a/configure.ac
+++ /dev/null
@@ -1,13 +0,0 @@
-AC_PREREQ([2.69])
-AC_INIT([platsch], [2019.12.0], [oss-tools@pengutronix.de])
-AC_CONFIG_SRCDIR([platsch.c])
-AM_INIT_AUTOMAKE([foreign dist-xz])
-
-AC_PROG_CC
-AC_PROG_MAKE_SET
-
-PKG_CHECK_MODULES([LIBDRM], [libdrm >= 2.4.112])
-
-AC_CONFIG_FILES([Makefile])
-
-AC_OUTPUT
diff --git a/meson.build b/meson.build
new file mode 100644
index 0000000..9f6be1e
--- /dev/null
+++ b/meson.build
@@ -0,0 +1,15 @@
+project('platsch', 'c')
+
+platsch_dep = dependency('libdrm', version : '>= 2.4.112', required : true)
+sources = ['platsch.c']
+
+# Define the headers
+headers = ['platsch.h']
+
+# Create the platsch executable
+executable('platsch',
+    sources,
+    dependencies: platsch_dep,
+    install: true,
+    include_directories: include_directories('.')
+)
-- 
2.34.1




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

* [OSS-Tools] [PATCH platsch V3 3/4] platsch: split into platsch and libplatsch
  2024-06-13  7:07 [OSS-Tools] [PATCH platsch V3 1/4] platsch: constify draw_buffer LI Qingwu
  2024-06-13  7:07 ` [OSS-Tools] [PATCH platsch V3 2/4] convert to meson build LI Qingwu
@ 2024-06-13  7:07 ` LI Qingwu
  2024-06-14 10:57   ` Ulrich Ölmann
  2024-06-13  7:07 ` [OSS-Tools] [PATCH platsch V3 4/4] Add spinner executable for boot animation and text show LI Qingwu
  2024-06-14  5:30 ` [OSS-Tools] [PATCH platsch V3 1/4] platsch: constify draw_buffer Ulrich Ölmann
  3 siblings, 1 reply; 9+ messages in thread
From: LI Qingwu @ 2024-06-13  7:07 UTC (permalink / raw)
  To: Qing-wu.Li, oss-tools, m.felsch

Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
---
 libplatsch.c | 590 +++++++++++++++++++++++++++++++++++++++++++++++++++
 libplatsch.h |  51 +++++
 meson.build  |  13 +-
 platsch.c    | 582 +-------------------------------------------------
 4 files changed, 658 insertions(+), 578 deletions(-)
 create mode 100644 libplatsch.c
 create mode 100644 libplatsch.h

diff --git a/libplatsch.c b/libplatsch.c
new file mode 100644
index 0000000..1d48e0e
--- /dev/null
+++ b/libplatsch.c
@@ -0,0 +1,590 @@
+/*
+ * Copyright (C) 2019 Pengutronix, Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
+ *
+ * Permission to use, copy, modify, and/or distribute this software
+ * for any purpose with or without fee is hereby granted.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
+ * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE
+ * AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ * Some code parts base on example code written in 2012 by David Herrmann
+ * <dh.herrmann@googlemail.com> and dedicated to the Public Domain. It was found
+ * in 2019 on
+ * https://raw.githubusercontent.com/dvdhrm/docs/master/drm-howto/modeset.c
+ */
+
+#include <assert.h>
+#include <ctype.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <getopt.h>
+#include <libgen.h>
+#include <stdarg.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "libplatsch.h"
+#include <drm_fourcc.h>
+#include <xf86drm.h>
+#include <xf86drmMode.h>
+
+#include "libplatsch.h"
+
+void redirect_stdfd(void)
+{
+	int devnull = open("/dev/null", O_RDWR, 0);
+
+	if (devnull < 0) {
+		error("Failed to open /dev/null: %m\n");
+		return;
+	}
+
+	close(STDIN_FILENO);
+	dup2(devnull, STDIN_FILENO);
+	close(STDOUT_FILENO);
+	dup2(devnull, STDOUT_FILENO);
+	close(STDERR_FILENO);
+	dup2(devnull, STDERR_FILENO);
+	close(devnull);
+}
+
+ssize_t readfull(int fd, void *buf, size_t count)
+{
+	ssize_t ret = 0, err;
+
+	while (count > 0) {
+		err = read(fd, buf, count);
+		if (err < 0)
+			return err;
+		else if (err > 0) {
+			buf += err;
+			count -= err;
+			ret += err;
+		} else {
+			return ret;
+		}
+	}
+
+	return ret;
+}
+
+void draw_buffer(struct modeset_dev *dev, const char *dir, const char *base)
+{
+	int fd_src;
+	char filename[128];
+	ssize_t size;
+	int ret;
+
+	/*
+	 * make it easy and load a raw file in the right format instead of
+	 * opening an (say) PNG and convert the image data to the right format.
+	 */
+	ret = snprintf(filename, sizeof(filename), "%s/%s-%ux%u-%s.bin", dir, base, dev->width,
+		       dev->height, dev->format->name);
+	if (ret >= sizeof(filename)) {
+		error("Failed to fit filename into buffer\n");
+		return;
+	}
+
+	fd_src = open(filename, O_RDONLY | O_CLOEXEC);
+	if (fd_src < 0) {
+		error("Failed to open %s: %m\n", filename);
+		return;
+	}
+
+	size = readfull(fd_src, dev->map, dev->size);
+	if (size < dev->size) {
+		if (size < 0)
+			error("Failed to read from %s: %m\n", filename);
+		else
+			error("Could only read %zd/%u bytes from %s\n", size, dev->size, filename);
+	}
+
+	ret = close(fd_src);
+	if (ret < 0) {
+		/* Nothing we can do about this, so just warn */
+		error("Failed to close image file\n");
+	}
+
+	return;
+}
+
+static struct modeset_dev *modeset_list = NULL;
+
+static int drmprepare_crtc(int fd, drmModeRes *res, drmModeConnector *conn, struct modeset_dev *dev)
+{
+	drmModeEncoder *enc;
+	unsigned int i, j;
+	int32_t crtc_id;
+	struct modeset_dev *iter;
+
+	/* first try the currently connected encoder+crtc */
+	if (conn->encoder_id) {
+		debug("connector #%d uses encoder #%d\n", conn->connector_id, conn->encoder_id);
+		enc = drmModeGetEncoder(fd, conn->encoder_id);
+		assert(enc);
+		assert(enc->encoder_id == conn->encoder_id);
+	} else {
+		debug("connector #%d has no active encoder\n", conn->connector_id);
+		enc = NULL;
+		dev->setmode = 1;
+	}
+
+	if (enc) {
+		if (enc->crtc_id) {
+			crtc_id = enc->crtc_id;
+			assert(crtc_id >= 0);
+
+			for (iter = modeset_list; iter; iter = iter->next) {
+				if (iter->crtc_id == crtc_id) {
+					crtc_id = -1;
+					break;
+				}
+			}
+
+			if (crtc_id > 0) {
+				debug("encoder #%d uses crtc #%d\n", enc->encoder_id, enc->crtc_id);
+				drmModeFreeEncoder(enc);
+				dev->crtc_id = crtc_id;
+				return 0;
+			} else {
+				debug("encoder #%d used crtc #%d, but that's in use\n",
+				      enc->encoder_id, iter->crtc_id);
+			}
+		} else {
+			debug("encoder #%d doesn't have an active crtc\n", enc->encoder_id);
+		}
+
+		drmModeFreeEncoder(enc);
+	}
+
+	/* If the connector is not currently bound to an encoder or if the
+	 * encoder+crtc is already used by another connector (actually unlikely
+	 * but let's be safe), iterate all other available encoders to find a
+	 * matching CRTC. */
+	for (i = 0; i < conn->count_encoders; ++i) {
+		enc = drmModeGetEncoder(fd, conn->encoders[i]);
+		if (!enc) {
+			error("Cannot retrieve encoder %u: %m\n", conn->encoders[i]);
+			continue;
+		}
+		assert(enc->encoder_id == conn->encoders[i]);
+
+		/* iterate all global CRTCs */
+		for (j = 0; j < res->count_crtcs; ++j) {
+			/* check whether this CRTC works with the encoder */
+			if (!(enc->possible_crtcs & (1 << j)))
+				continue;
+
+			/* check that no other device already uses this CRTC */
+			crtc_id = res->crtcs[j];
+			for (iter = modeset_list; iter; iter = iter->next) {
+				if (iter->crtc_id == crtc_id) {
+					crtc_id = -1;
+					break;
+				}
+			}
+
+			/* we have found a CRTC, so save it and return */
+			if (crtc_id >= 0) {
+				debug("encoder #%d will use crtc #%d\n", enc->encoder_id, crtc_id);
+				drmModeFreeEncoder(enc);
+				dev->crtc_id = crtc_id;
+				return 0;
+			}
+		}
+		drmModeFreeEncoder(enc);
+	}
+
+	error("Cannot find suitable CRTC for connector #%u\n", conn->connector_id);
+	return -ENOENT;
+}
+
+static int modeset_create_fb(int fd, struct modeset_dev *dev)
+{
+	struct drm_mode_create_dumb creq;
+	struct drm_mode_destroy_dumb dreq;
+	struct drm_mode_map_dumb mreq;
+	int ret;
+
+	/* create dumb buffer */
+	memset(&creq, 0, sizeof(creq));
+	creq.width = dev->width;
+	creq.height = dev->height;
+	creq.bpp = dev->format->bpp;
+	ret = drmIoctl(fd, DRM_IOCTL_MODE_CREATE_DUMB, &creq);
+	if (ret < 0) {
+		error("Cannot create dumb buffer: %m\n");
+		return -errno;
+	}
+	dev->stride = creq.pitch;
+	dev->size = creq.size;
+	dev->handle = creq.handle;
+
+	/* create framebuffer object for the dumb-buffer */
+	ret = drmModeAddFB2(fd, dev->width, dev->height,
+			    dev->format->format,
+			    (uint32_t[4]){ dev->handle, },
+			    (uint32_t[4]){ dev->stride, },
+			    (uint32_t[4]){ 0, },
+			    &dev->fb_id, 0);
+	if (ret) {
+		ret = -errno;
+		error("Cannot create framebuffer: %m\n");
+		goto err_destroy;
+	}
+
+	/* prepare buffer for memory mapping */
+	memset(&mreq, 0, sizeof(mreq));
+	mreq.handle = dev->handle;
+	ret = drmIoctl(fd, DRM_IOCTL_MODE_MAP_DUMB, &mreq);
+	if (ret) {
+		ret = -errno;
+		error("Cannot get mmap offset: %m\n");
+		goto err_fb;
+	}
+
+	/* perform actual memory mapping */
+	dev->map = mmap(0, dev->size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, mreq.offset);
+	if (dev->map == MAP_FAILED) {
+		ret = -errno;
+		error("Cannot mmap dumb buffer: %m\n");
+		goto err_fb;
+	}
+
+	/*
+	 * Clear the framebuffer. Normally it's overwritten later with some
+	 * image data, but in case this fails, initialize to all-black.
+	 */
+	memset(dev->map, 0x0, dev->size);
+
+	return 0;
+
+err_fb:
+	drmModeRmFB(fd, dev->fb_id);
+err_destroy:
+	memset(&dreq, 0, sizeof(dreq));
+	dreq.handle = dev->handle;
+	drmIoctl(fd, DRM_IOCTL_MODE_DESTROY_DUMB, &dreq);
+	return ret;
+}
+
+/* Returns lowercase connector type names with '_' for '-' */
+static char *get_normalized_conn_type_name(uint32_t connector_type)
+{
+	int i;
+	const char *connector_name;
+	char *normalized_name;
+
+	connector_name = drmModeGetConnectorTypeName(connector_type);
+	if (!connector_name)
+		return NULL;
+
+	normalized_name = strdup(connector_name);
+
+	for (i = 0; normalized_name[i]; i++) {
+		normalized_name[i] = tolower(normalized_name[i]);
+		if (normalized_name[i] == '-')
+			normalized_name[i] = '_';
+	}
+
+	return normalized_name;
+}
+
+static const struct platsch_format *platsch_format_find(const char *name)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(platsch_formats); i++)
+		if (!strcmp(platsch_formats[i].name, name))
+			return &platsch_formats[i];
+
+	return NULL;
+}
+
+static int set_env_connector_mode(drmModeConnector *conn, struct modeset_dev *dev)
+{
+	int ret, i = 0;
+	u_int32_t width = 0, height = 0;
+	const char *mode;
+	char *connector_type_name, mode_env_name[32], fmt_specifier[32] = "";
+	const struct platsch_format *format = NULL;
+
+	connector_type_name = get_normalized_conn_type_name(conn->connector_type);
+	if (!connector_type_name) {
+		error("could not look up name for connector type %u\n", conn->connector_type);
+		goto fallback;
+	}
+
+	ret = snprintf(mode_env_name, sizeof(mode_env_name), "platsch_%s%u_mode",
+		       connector_type_name, conn->connector_type_id);
+	free(connector_type_name);
+	if (ret >= sizeof(mode_env_name)) {
+		error("failed to fit platsch env mode variable name into buffer\n");
+		return -EFAULT;
+	}
+
+	/* check for connector mode configuration in environment */
+	debug("looking up %s env variable\n", mode_env_name);
+	mode = getenv(mode_env_name);
+	if (!mode)
+		goto fallback;
+
+	/* format suffix is optional */
+	ret = sscanf(mode, "%ux%u@%s", &width, &height, fmt_specifier);
+	if (ret < 2) {
+		error("error while scanning %s for mode\n", mode_env_name);
+		return -EFAULT;
+	}
+
+	/* use first mode matching given resolution */
+	for (i = 0; i < conn->count_modes; i++) {
+		drmModeModeInfo mode = conn->modes[i];
+		if (mode.hdisplay == width && mode.vdisplay == height) {
+			memcpy(&dev->mode, &mode, sizeof(dev->mode));
+			dev->width = width;
+			dev->height = height;
+			break;
+		}
+	}
+
+	if (i == conn->count_modes) {
+		error("no mode available matching %ux%u\n", width, height);
+		return -ENOENT;
+	}
+
+	format = platsch_format_find(fmt_specifier);
+	if (!format) {
+		if (strlen(fmt_specifier))
+			error("unknown format specifier %s\n", fmt_specifier);
+		goto fallback_format;
+	}
+
+	dev->format = format;
+
+	return 0;
+
+fallback:
+	memcpy(&dev->mode, &conn->modes[0], sizeof(dev->mode));
+	dev->width = conn->modes[0].hdisplay;
+	dev->height = conn->modes[0].vdisplay;
+	debug("using default mode for connector #%u\n", conn->connector_id);
+
+fallback_format:
+	dev->format = &platsch_formats[0];
+	debug("using default format %s for connector #%u\n", dev->format->name, conn->connector_id);
+
+	return 0;
+}
+
+static int drmprepare_connector(int fd, drmModeRes *res, drmModeConnector *conn,
+				struct modeset_dev *dev)
+{
+	int ret;
+
+	/* check if a monitor is connected */
+	if (conn->connection != DRM_MODE_CONNECTED) {
+		error("Ignoring unused connector #%u\n", conn->connector_id);
+		return -ENOENT;
+	}
+
+	/* check if there is at least one valid mode */
+	if (conn->count_modes == 0) {
+		error("no valid mode for connector #%u\n", conn->connector_id);
+		return -EFAULT;
+	}
+
+	/* configure mode information in our device structure */
+	ret = set_env_connector_mode(conn, dev);
+	if (ret) {
+		error("no valid mode for connector #%u\n", conn->connector_id);
+		return ret;
+	}
+	debug("mode for connector #%u is %ux%u@%s\n", conn->connector_id, dev->width, dev->height,
+	      dev->format->name);
+
+	/* find a crtc for this connector */
+	ret = drmprepare_crtc(fd, res, conn, dev);
+	if (ret) {
+		error("no valid crtc for connector #%u\n", conn->connector_id);
+		return ret;
+	}
+
+	/* create a framebuffer for this CRTC */
+	ret = modeset_create_fb(fd, dev);
+	if (ret) {
+		error("cannot create framebuffer for connector #%u\n", conn->connector_id);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int drmprepare(int fd)
+{
+	drmModeRes *res;
+	drmModeConnector *conn;
+	unsigned int i;
+	struct modeset_dev *dev;
+	int ret;
+
+	/* retrieve resources */
+	res = drmModeGetResources(fd);
+	if (!res) {
+		error("cannot retrieve DRM resources: %m\n");
+		return -errno;
+	}
+
+	debug("Found %d connectors\n", res->count_connectors);
+
+	/* iterate all connectors */
+	for (i = 0; i < res->count_connectors; ++i) {
+		/* get information for each connector */
+		conn = drmModeGetConnector(fd, res->connectors[i]);
+		if (!conn) {
+			error("Cannot retrieve DRM connector #%u: %m\n", res->connectors[i]);
+			continue;
+		}
+		assert(conn->connector_id == res->connectors[i]);
+
+		debug("Connector #%u has type %s\n", conn->connector_id,
+		      drmModeGetConnectorTypeName(conn->connector_type));
+
+		/* create a device structure */
+		dev = malloc(sizeof(*dev));
+		if (!dev) {
+			error("Cannot allocate memory for connector #%u: %m\n", res->connectors[i]);
+			continue;
+		}
+		memset(dev, 0, sizeof(*dev));
+		dev->conn_id = conn->connector_id;
+
+		ret = drmprepare_connector(fd, res, conn, dev);
+		if (ret) {
+			if (ret != -ENOENT) {
+				error("Cannot setup device for connector #%u: %m\n",
+				      res->connectors[i]);
+			}
+			free(dev);
+			drmModeFreeConnector(conn);
+			continue;
+		}
+
+		/* free connector data and link device into global list */
+		drmModeFreeConnector(conn);
+		dev->next = modeset_list;
+		modeset_list = dev;
+	}
+
+	/* free resources again */
+	drmModeFreeResources(res);
+	return 0;
+}
+
+static int drmfd;
+
+struct modeset_dev *init(void)
+{
+	static char drmdev[128];
+	int ret = 0, i;
+
+	for (i = 0; i < 64; i++) {
+		struct drm_mode_card_res res = {0};
+
+		/*
+		 * XXX: Maybe use drmOpen instead?
+		 * (Where should name/busid come from?)
+		 * XXX: Loop through drm devices to find one with connectors.
+		 */
+		ret = snprintf(drmdev, sizeof(drmdev), DRM_DEV_NAME, DRM_DIR_NAME, i);
+		if (ret >= sizeof(drmdev)) {
+			error("Huh, device name overflowed buffer\n");
+			goto execinit;
+		}
+
+		drmfd = open(drmdev, O_RDWR | O_CLOEXEC, 0);
+		if (drmfd < 0) {
+			error("Failed to open drm device: %m\n");
+			goto execinit;
+		}
+
+		ret = drmIoctl(drmfd, DRM_IOCTL_MODE_GETRESOURCES, &res);
+		if (ret < 0) {
+			close(drmfd);
+			continue;
+		} else {
+			/* Device found */
+			break;
+		}
+	}
+
+	if (i == 64) {
+		error("No suitable DRM device found\n");
+		goto execinit;
+	}
+
+	ret = drmprepare(drmfd);
+	if (ret) {
+		error("Failed to prepare DRM device\n");
+		goto execinit;
+	}
+
+	return modeset_list;
+
+execinit:
+	return NULL;
+}
+
+void update_display(struct modeset_dev *dev)
+{
+	int ret = 0;
+
+	if (dev->setmode) {
+		ret = drmModeSetCrtc(drmfd, dev->crtc_id, dev->fb_id, 0, 0, &dev->conn_id, 1,
+				     &dev->mode);
+		if (ret) {
+			error("Cannot set CRTC for connector #%u: %m\n", dev->conn_id);
+		}
+		dev->setmode = 0;
+	} else {
+		ret = drmModePageFlip(drmfd, dev->crtc_id, dev->fb_id, 0, NULL);
+		if (ret) {
+			error("Page flip failed on connector #%u: %m\n", dev->conn_id);
+		}
+	}
+}
+
+void draw(struct modeset_dev *dev, const char *dir, const char *base)
+{
+	draw_buffer(dev, dir, base);
+	update_display(dev);
+}
+
+void finish(void) { drmDropMaster(drmfd); }
+
+void deinit(void)
+{
+	struct modeset_dev *iter;
+
+	for (iter = modeset_list; iter; iter = iter->next) {
+		if (iter->map) {
+			munmap(iter->map, iter->size);
+		}
+		if (iter->fb_id) {
+			drmModeRmFB(drmfd, iter->fb_id);
+		}
+		free(iter);
+	}
+}
diff --git a/libplatsch.h b/libplatsch.h
new file mode 100644
index 0000000..77f74a9
--- /dev/null
+++ b/libplatsch.h
@@ -0,0 +1,51 @@
+#ifndef __LIBPLATSCH_H__
+#define __LIBPLATSCH_H__
+
+#include <xf86drmMode.h>
+#include <drm_fourcc.h>
+#include "libplatsch.h"
+#include <unistd.h>
+
+
+
+#define debug(fmt, ...) printf("%s:%d: " fmt, __func__, __LINE__, ##__VA_ARGS__)
+#define error(fmt, ...) fprintf(stderr, fmt, ##__VA_ARGS__)
+
+#define ARRAY_SIZE(a) (sizeof(a) / sizeof(*a))
+
+struct platsch_format {
+	uint32_t format;
+	uint32_t bpp;
+	const char *name;
+};
+
+struct modeset_dev {
+	struct modeset_dev *next;
+
+	uint32_t width;
+	uint32_t height;
+	uint32_t stride;
+	uint32_t size;
+	const struct platsch_format *format;
+	uint32_t handle;
+	void *map;
+	bool setmode;
+	drmModeModeInfo mode;
+	uint32_t fb_id;
+	uint32_t conn_id;
+	uint32_t crtc_id;
+};
+
+static const struct platsch_format platsch_formats[] = {
+	{ DRM_FORMAT_RGB565, 16, "RGB565" }, /* default */
+	{ DRM_FORMAT_XRGB8888, 32, "XRGB8888" },
+};
+
+ssize_t readfull(int fd, void *buf, size_t count);
+struct modeset_dev *init(void);
+void draw(struct modeset_dev *dev, const char *dir, const char *base);
+void finish(void);
+void redirect_stdfd(void);
+void update_display(struct modeset_dev *dev);
+
+#endif
\ No newline at end of file
diff --git a/meson.build b/meson.build
index 9f6be1e..dc55238 100644
--- a/meson.build
+++ b/meson.build
@@ -1,15 +1,18 @@
 project('platsch', 'c')
 
 platsch_dep = dependency('libdrm', version : '>= 2.4.112', required : true)
-sources = ['platsch.c']
+sources = ['libplatsch.c']
 
-# Define the headers
-headers = ['platsch.h']
 
-# Create the platsch executable
-executable('platsch',
+libplatsch = static_library('libplatsch',
     sources,
     dependencies: platsch_dep,
+)
+
+executable('platsch',
+    'platsch.c',
+    dependencies: platsch_dep,
+    link_with: libplatsch,
     install: true,
     include_directories: include_directories('.')
 )
diff --git a/platsch.c b/platsch.c
index 1aaa8d5..917fec0 100644
--- a/platsch.c
+++ b/platsch.c
@@ -19,528 +19,15 @@
  * https://raw.githubusercontent.com/dvdhrm/docs/master/drm-howto/modeset.c
  */
 
-#include <assert.h>
-#include <ctype.h>
-#include <errno.h>
 #include <getopt.h>
 #include <libgen.h>
-#include <stdarg.h>
-#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <sys/mman.h>
-#include <fcntl.h>
 
-#include <xf86drm.h>
-#include <xf86drmMode.h>
-#include <drm_fourcc.h>
+#include "libplatsch.h"
 
-#define debug(fmt, ...) printf("%s:%d: " fmt, __func__, __LINE__, ##__VA_ARGS__)
-#define error(fmt, ...) fprintf(stderr, fmt, ##__VA_ARGS__)
-
-#define ARRAY_SIZE(a) (sizeof(a) / sizeof(*a))
-
-struct platsch_format {
-	uint32_t format;
-	uint32_t bpp;
-	const char *name;
-};
-
-static const struct platsch_format platsch_formats[] = {
-	{ DRM_FORMAT_RGB565, 16, "RGB565" }, /* default */
-	{ DRM_FORMAT_XRGB8888, 32, "XRGB8888" },
-};
-
-void redirect_stdfd(void)
-{
-	int devnull = open("/dev/null", O_RDWR, 0);
-
-	if (devnull < 0) {
-		error("Failed to open /dev/null: %m\n");
-		return;
-	}
-
-	close(STDIN_FILENO);
-	dup2(devnull, STDIN_FILENO);
-	close(STDOUT_FILENO);
-	dup2(devnull, STDOUT_FILENO);
-	close(STDERR_FILENO);
-	dup2(devnull, STDERR_FILENO);
-	close(devnull);
-}
-
-ssize_t readfull(int fd, void *buf, size_t count)
-{
-	ssize_t ret = 0, err;
-
-	while (count > 0) {
-		err = read(fd, buf, count);
-		if (err < 0)
-			return err;
-		else if (err > 0) {
-			buf += err;
-			count -= err;
-			ret += err;
-		} else {
-			return ret;
-		}
-	}
-
-	return ret;
-}
-
-struct modeset_dev {
-	struct modeset_dev *next;
-
-	uint32_t width;
-	uint32_t height;
-	uint32_t stride;
-	uint32_t size;
-	const struct platsch_format *format;
-	uint32_t handle;
-	void *map;
-
-	bool setmode;
-	drmModeModeInfo mode;
-	uint32_t fb_id;
-	uint32_t conn_id;
-	uint32_t crtc_id;
-};
-
-void draw_buffer(struct modeset_dev *dev, const char *dir, const char *base)
-{
-	int fd_src;
-	char filename[128];
-	ssize_t size;
-	int ret;
-
-	/*
-	 * make it easy and load a raw file in the right format instead of
-	 * opening an (say) PNG and convert the image data to the right format.
-	 */
-	ret = snprintf(filename, sizeof(filename),
-		       "%s/%s-%ux%u-%s.bin",
-		       dir, base, dev->width, dev->height, dev->format->name);
-	if (ret >= sizeof(filename)) {
-		error("Failed to fit filename into buffer\n");
-		return;
-	}
-
-	fd_src = open(filename, O_RDONLY | O_CLOEXEC);
-	if (fd_src < 0) {
-		error("Failed to open %s: %m\n", filename);
-		return;
-	}
-
-	size = readfull(fd_src, dev->map, dev->size);
-	if (size < dev->size) {
-		if (size < 0)
-			error("Failed to read from %s: %m\n", filename);
-		else
-			error("Could only read %zd/%u bytes from %s\n",
-			      size, dev->size, filename);
-	}
-
-	ret = close(fd_src);
-	if (ret < 0) {
-		/* Nothing we can do about this, so just warn */
-		error("Failed to close image file\n");
-	}
-
-	return;
-}
-
-static struct modeset_dev *modeset_list = NULL;
-
-static int drmprepare_crtc(int fd, drmModeRes *res, drmModeConnector *conn,
-			   struct modeset_dev *dev)
-{
-	drmModeEncoder *enc;
-	unsigned int i, j;
-	int32_t crtc_id;
-	struct modeset_dev *iter;
-
-	/* first try the currently connected encoder+crtc */
-	if (conn->encoder_id) {
-		debug("connector #%d uses encoder #%d\n", conn->connector_id,
-		      conn->encoder_id);
-		enc = drmModeGetEncoder(fd, conn->encoder_id);
-		assert(enc);
-		assert(enc->encoder_id == conn->encoder_id);
-	} else {
-		debug("connector #%d has no active encoder\n",
-		      conn->connector_id);
-		enc = NULL;
-		dev->setmode = 1;
-	}
-
-	if (enc) {
-		if (enc->crtc_id) {
-			crtc_id = enc->crtc_id;
-			assert(crtc_id >= 0);
-
-			for (iter = modeset_list; iter; iter = iter->next) {
-				if (iter->crtc_id == crtc_id) {
-					crtc_id = -1;
-					break;
-				}
-			}
-
-			if (crtc_id > 0) {
-				debug("encoder #%d uses crtc #%d\n",
-				      enc->encoder_id, enc->crtc_id);
-				drmModeFreeEncoder(enc);
-				dev->crtc_id = crtc_id;
-				return 0;
-			} else {
-				debug("encoder #%d used crtc #%d, but that's in use\n",
-				      enc->encoder_id, iter->crtc_id);
-			}
-		} else {
-			debug("encoder #%d doesn't have an active crtc\n",
-			      enc->encoder_id);
-		}
-
-		drmModeFreeEncoder(enc);
-	}
-
-	/* If the connector is not currently bound to an encoder or if the
-	 * encoder+crtc is already used by another connector (actually unlikely
-	 * but let's be safe), iterate all other available encoders to find a
-	 * matching CRTC. */
-	for (i = 0; i < conn->count_encoders; ++i) {
-		enc = drmModeGetEncoder(fd, conn->encoders[i]);
-		if (!enc) {
-			error("Cannot retrieve encoder %u: %m\n",
-			      conn->encoders[i]);
-			continue;
-		}
-		assert(enc->encoder_id == conn->encoders[i]);
-
-		/* iterate all global CRTCs */
-		for (j = 0; j < res->count_crtcs; ++j) {
-			/* check whether this CRTC works with the encoder */
-			if (!(enc->possible_crtcs & (1 << j)))
-				continue;
-
-			/* check that no other device already uses this CRTC */
-			crtc_id = res->crtcs[j];
-			for (iter = modeset_list; iter; iter = iter->next) {
-				if (iter->crtc_id == crtc_id) {
-					crtc_id = -1;
-					break;
-				}
-			}
-
-			/* we have found a CRTC, so save it and return */
-			if (crtc_id >= 0) {
-				debug("encoder #%d will use crtc #%d\n",
-				      enc->encoder_id, crtc_id);
-				drmModeFreeEncoder(enc);
-				dev->crtc_id = crtc_id;
-				return 0;
-			}
-
-		}
-		drmModeFreeEncoder(enc);
-	}
-
-	error("Cannot find suitable CRTC for connector #%u\n",
-	      conn->connector_id);
-	return -ENOENT;
-}
-
-static int modeset_create_fb(int fd, struct modeset_dev *dev)
-{
-	struct drm_mode_create_dumb creq;
-	struct drm_mode_destroy_dumb dreq;
-	struct drm_mode_map_dumb mreq;
-	int ret;
-
-	/* create dumb buffer */
-	memset(&creq, 0, sizeof(creq));
-	creq.width = dev->width;
-	creq.height = dev->height;
-	creq.bpp = dev->format->bpp;
-	ret = drmIoctl(fd, DRM_IOCTL_MODE_CREATE_DUMB, &creq);
-	if (ret < 0) {
-		error("Cannot create dumb buffer: %m\n");
-		return -errno;
-	}
-	dev->stride = creq.pitch;
-	dev->size = creq.size;
-	dev->handle = creq.handle;
-
-	/* create framebuffer object for the dumb-buffer */
-	ret = drmModeAddFB2(fd, dev->width, dev->height,
-			    dev->format->format,
-			    (uint32_t[4]){ dev->handle, },
-			    (uint32_t[4]){ dev->stride, },
-			    (uint32_t[4]){ 0, },
-			    &dev->fb_id, 0);
-	if (ret) {
-		ret = -errno;
-		error("Cannot create framebuffer: %m\n");
-		goto err_destroy;
-	}
-
-	/* prepare buffer for memory mapping */
-	memset(&mreq, 0, sizeof(mreq));
-	mreq.handle = dev->handle;
-	ret = drmIoctl(fd, DRM_IOCTL_MODE_MAP_DUMB, &mreq);
-	if (ret) {
-		ret = -errno;
-		error("Cannot get mmap offset: %m\n");
-		goto err_fb;
-	}
-
-	/* perform actual memory mapping */
-	dev->map = mmap(0, dev->size, PROT_READ | PROT_WRITE, MAP_SHARED,
-			fd, mreq.offset);
-	if (dev->map == MAP_FAILED) {
-		ret = -errno;
-		error("Cannot mmap dumb buffer: %m\n");
-		goto err_fb;
-	}
-
-	/*
-	 * Clear the framebuffer. Normally it's overwritten later with some
-	 * image data, but in case this fails, initialize to all-black.
-	 */
-	memset(dev->map, 0x0, dev->size);
-
-	return 0;
-
-err_fb:
-	drmModeRmFB(fd, dev->fb_id);
-err_destroy:
-	memset(&dreq, 0, sizeof(dreq));
-	dreq.handle = dev->handle;
-	drmIoctl(fd, DRM_IOCTL_MODE_DESTROY_DUMB, &dreq);
-	return ret;
-}
-
-/* Returns lowercase connector type names with '_' for '-' */
-static char *get_normalized_conn_type_name(uint32_t connector_type)
-{
-	int i;
-	const char *connector_name;
-	char *normalized_name;
-
-	connector_name = drmModeGetConnectorTypeName(connector_type);
-	if (!connector_name)
-		return NULL;
-
-	normalized_name = strdup(connector_name);
-
-	for (i = 0; normalized_name[i]; i++) {
-		normalized_name[i] = tolower(normalized_name[i]);
-		if (normalized_name[i] == '-')
-			normalized_name[i] = '_';
-	}
-
-	return normalized_name;
-}
-
-static const struct platsch_format *platsch_format_find(const char *name)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(platsch_formats); i++)
-		if (!strcmp(platsch_formats[i].name, name))
-			return &platsch_formats[i];
-
-	return NULL;
-}
-
-static int set_env_connector_mode(drmModeConnector *conn,
-				  struct modeset_dev *dev)
-{
-	int ret, i = 0;
-	u_int32_t width = 0, height = 0;
-	const char *mode;
-	char *connector_type_name, mode_env_name[32], fmt_specifier[32] = "";
-	const struct platsch_format *format = NULL;
-
-	connector_type_name = get_normalized_conn_type_name(conn->connector_type);
-	if (!connector_type_name) {
-		error("could not look up name for connector type %u\n",
-		      conn->connector_type);
-		goto fallback;
-	}
-
-	ret = snprintf(mode_env_name, sizeof(mode_env_name), "platsch_%s%u_mode",
-		       connector_type_name, conn->connector_type_id);
-	free(connector_type_name);
-	if (ret >= sizeof(mode_env_name)) {
-		error("failed to fit platsch env mode variable name into buffer\n");
-		return -EFAULT;
-	}
-
-	/* check for connector mode configuration in environment */
-	debug("looking up %s env variable\n", mode_env_name);
-	mode = getenv(mode_env_name);
-	if (!mode)
-		goto fallback;
-
-	/* format suffix is optional */
-	ret = sscanf(mode, "%ux%u@%s", &width, &height, fmt_specifier);
-	if (ret < 2) {
-		error("error while scanning %s for mode\n", mode_env_name);
-		return -EFAULT;
-	}
-
-	/* use first mode matching given resolution */
-	for (i = 0; i < conn->count_modes; i++) {
-		drmModeModeInfo mode = conn->modes[i];
-		if (mode.hdisplay == width && mode.vdisplay == height) {
-			memcpy(&dev->mode, &mode, sizeof(dev->mode));
-			dev->width = width;
-			dev->height = height;
-			break;
-		}
-	}
-
-	if (i == conn->count_modes) {
-		error("no mode available matching %ux%u\n", width, height);
-		return -ENOENT;
-	}
-
-	format = platsch_format_find(fmt_specifier);
-	if (!format) {
-		if (strlen(fmt_specifier))
-			error("unknown format specifier %s\n", fmt_specifier);
-		goto fallback_format;
-	}
-
-	dev->format = format;
-
-	return 0;
-
-fallback:
-	memcpy(&dev->mode, &conn->modes[0], sizeof(dev->mode));
-	dev->width = conn->modes[0].hdisplay;
-	dev->height = conn->modes[0].vdisplay;
-	debug("using default mode for connector #%u\n", conn->connector_id);
-
-fallback_format:
-	dev->format = &platsch_formats[0];
-	debug("using default format %s for connector #%u\n", dev->format->name,
-	      conn->connector_id);
-
-	return 0;
-}
-
-static int drmprepare_connector(int fd, drmModeRes *res, drmModeConnector *conn,
-				struct modeset_dev *dev)
-{
-	int ret;
-
-	/* check if a monitor is connected */
-	if (conn->connection != DRM_MODE_CONNECTED) {
-		error("Ignoring unused connector #%u\n", conn->connector_id);
-		return -ENOENT;
-	}
-
-	/* check if there is at least one valid mode */
-	if (conn->count_modes == 0) {
-		error("no valid mode for connector #%u\n", conn->connector_id);
-		return -EFAULT;
-	}
-
-	/* configure mode information in our device structure */
-	ret = set_env_connector_mode(conn, dev);
-	if (ret) {
-		error("no valid mode for connector #%u\n", conn->connector_id);
-		return ret;
-	}
-	debug("mode for connector #%u is %ux%u@%s\n",
-	      conn->connector_id, dev->width, dev->height, dev->format->name);
-
-	/* find a crtc for this connector */
-	ret = drmprepare_crtc(fd, res, conn, dev);
-	if (ret) {
-		error("no valid crtc for connector #%u\n", conn->connector_id);
-		return ret;
-	}
-
-	/* create a framebuffer for this CRTC */
-	ret = modeset_create_fb(fd, dev);
-	if (ret) {
-		error("cannot create framebuffer for connector #%u\n",
-		      conn->connector_id);
-		return ret;
-	}
-
-	return 0;
-}
-
-static int drmprepare(int fd)
-{
-	drmModeRes *res;
-	drmModeConnector *conn;
-	unsigned int i;
-	struct modeset_dev *dev;
-	int ret;
-
-	/* retrieve resources */
-	res = drmModeGetResources(fd);
-	if (!res) {
-		error("cannot retrieve DRM resources: %m\n");
-		return -errno;
-	}
-
-	debug("Found %d connectors\n", res->count_connectors);
-
-	/* iterate all connectors */
-	for (i = 0; i < res->count_connectors; ++i) {
-		/* get information for each connector */
-		conn = drmModeGetConnector(fd, res->connectors[i]);
-		if (!conn) {
-			error("Cannot retrieve DRM connector #%u: %m\n",
-				res->connectors[i]);
-			continue;
-		}
-		assert(conn->connector_id == res->connectors[i]);
-
-		debug("Connector #%u has type %s\n", conn->connector_id,
-		      drmModeGetConnectorTypeName(conn->connector_type));
-
-		/* create a device structure */
-		dev = malloc(sizeof(*dev));
-		if (!dev) {
-			error("Cannot allocate memory for connector #%u: %m\n",
-			      res->connectors[i]);
-			continue;
-		}
-		memset(dev, 0, sizeof(*dev));
-		dev->conn_id = conn->connector_id;
-
-		ret = drmprepare_connector(fd, res, conn, dev);
-		if (ret) {
-			if (ret != -ENOENT) {
-				error("Cannot setup device for connector #%u: %m\n",
-				      res->connectors[i]);
-			}
-			free(dev);
-			drmModeFreeConnector(conn);
-			continue;
-		}
-
-		/* free connector data and link device into global list */
-		drmModeFreeConnector(conn);
-		dev->next = modeset_list;
-		modeset_list = dev;
-	}
-
-	/* free resources again */
-	drmModeFreeResources(res);
-	return 0;
-}
 
 static struct option longopts[] =
 {
@@ -561,14 +48,12 @@ static void usage(const char *prog)
 int main(int argc, char *argv[])
 {
 	char **initsargv;
-	int drmfd;
-	char drmdev[128];
 	struct modeset_dev *iter;
 	bool pid1 = getpid() == 1;
 	const char *dir = "/usr/share/platsch";
 	const char *base = "splash";
 	const char *env;
-	int ret = 0, c, i;
+	int ret = 0, c;
 
 	env = getenv("platsch_directory");
 	if (env)
@@ -580,7 +65,7 @@ int main(int argc, char *argv[])
 
 	if (!pid1) {
 		while ((c = getopt_long(argc, argv, "hd:b:", longopts, NULL)) != EOF) {
-			switch(c) {
+			switch (c) {
 			case 'd':
 				dir = optarg;
 				break;
@@ -604,67 +89,18 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	for (i = 0; i < 64; i++) {
-		struct drm_mode_card_res res = {0};
-
-		/*
-		 * XXX: Maybe use drmOpen instead?
-		 * (Where should name/busid come from?)
-		 * XXX: Loop through drm devices to find one with connectors.
-		 */
-		ret = snprintf(drmdev, sizeof(drmdev), DRM_DEV_NAME, DRM_DIR_NAME, i);
-		if (ret >= sizeof(drmdev)) {
-			error("Huh, device name overflowed buffer\n");
-			goto execinit;
-		}
-
-		drmfd = open(drmdev, O_RDWR | O_CLOEXEC, 0);
-		if (drmfd < 0) {
-			error("Failed to open drm device: %m\n");
-			goto execinit;
-		}
-
-		ret = drmIoctl(drmfd, DRM_IOCTL_MODE_GETRESOURCES, &res);
-		if (ret < 0) {
-			close(drmfd);
-			continue;
-		} else {
-			/* Device found */
-			break;
-		}
+	struct modeset_dev *modeset_list = init();
+	if (!modeset_list) {
+		error("Failed to initialize modeset\n");
+		return EXIT_FAILURE;
 	}
 
-	ret = drmprepare(drmfd);
-	assert(!ret);
-
 	for (iter = modeset_list; iter; iter = iter->next) {
-
-		/* draw first then set the mode */
-		draw_buffer(iter, dir, base);
-
-		if (iter->setmode) {
-			debug("set crtc\n");
-
-			ret = drmModeSetCrtc(drmfd, iter->crtc_id, iter->fb_id,
-					     0, 0, &iter->conn_id, 1, &iter->mode);
-			if (ret)
-				error("Cannot set CRTC for connector #%u: %m\n",
-				      iter->conn_id);
-		} else {
-			debug("page flip\n");
-			ret = drmModePageFlip(drmfd, iter->crtc_id, iter->fb_id,
-					      0, NULL);
-			if (ret)
-				error("Page flip failed on connector #%u: %m\n",
-				      iter->conn_id);
-		}
+		draw(iter, dir, base);
 	}
 
-	ret = drmDropMaster(drmfd);
-	if (ret)
-		error("Failed to drop master on drm device\n");
+	finish();
 
-execinit:
 	if (pid1) {
 		ret = fork();
 		if (ret < 0) {
-- 
2.34.1




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

* [OSS-Tools] [PATCH platsch V3 4/4] Add spinner executable for boot animation and text show
  2024-06-13  7:07 [OSS-Tools] [PATCH platsch V3 1/4] platsch: constify draw_buffer LI Qingwu
  2024-06-13  7:07 ` [OSS-Tools] [PATCH platsch V3 2/4] convert to meson build LI Qingwu
  2024-06-13  7:07 ` [OSS-Tools] [PATCH platsch V3 3/4] platsch: split into platsch and libplatsch LI Qingwu
@ 2024-06-13  7:07 ` LI Qingwu
  2024-06-14  5:30 ` [OSS-Tools] [PATCH platsch V3 1/4] platsch: constify draw_buffer Ulrich Ölmann
  3 siblings, 0 replies; 9+ messages in thread
From: LI Qingwu @ 2024-06-13  7:07 UTC (permalink / raw)
  To: Qing-wu.Li, oss-tools, m.felsch

This commit introduces a new executable, spinner,
    which supports two types of animations for boot sequences:
    1 static PNG and text support.
    2 rotates square PNG images per frame
    3 shows a sequence of square images from a strip of PNG images.

Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
---
 README.rst        |  39 ++++++
 meson.build       |  24 ++++
 meson_options.txt |   1 +
 spinner.c         | 297 ++++++++++++++++++++++++++++++++++++++++++++++
 spinner_conf.c    |  69 +++++++++++
 spinner_conf.h    |  35 ++++++
 6 files changed, 465 insertions(+)
 create mode 100644 meson_options.txt
 create mode 100644 spinner.c
 create mode 100644 spinner_conf.c
 create mode 100644 spinner_conf.h

diff --git a/README.rst b/README.rst
index f1c0812..862186f 100644
--- a/README.rst
+++ b/README.rst
@@ -149,3 +149,42 @@ Compiling Instructions
 
     meson setup build
     meson compile -C build
+
+
+Spinner - Splash Screen with Animation
+======================================
+
+The `spinner` executable is designed to provide boot animations. It supports two types of animations:
+
+1. **static PNG and text support**: Show a static png image and text.
+1. **Square PNG Rotation Animation**: Rotates a square PNG image.
+2. **Sequence Move Rectangle Animation**: Displays a sequence of square images from a strip of PNG images.
+
+spinner Configuration
+---------------------
+
+The configuration for the `spinner` executable is read from a configuration file,
+with a default path of `/usr/share/platsch/spinner.conf`.
+The directory of the configuration file can be set via the `platsch_directory` environment variable.
+
+
+spinner Configuration
+---------------------
+
+Here is a sample  of a configuration file (`spinner.conf`):
+
+.. code-block:: ini
+
+    symbol="/usr/share/platsch/Spinner.png"
+    fps=20
+    # frames=0 means infinite
+    frames=0
+    text=""
+    text_x=350
+    text_y=400
+    text_font="Sans"
+    text_size=30
+
+Set text to empty if you don't want to display any text.
+Set the symbol to empty if you don't want to display any animation.
+The background image is indicated by ``--directory`` and ``--basename``.
diff --git a/meson.build b/meson.build
index dc55238..79bfd17 100644
--- a/meson.build
+++ b/meson.build
@@ -16,3 +16,27 @@ executable('platsch',
     install: true,
     include_directories: include_directories('.')
 )
+
+# Create the spinner executable if SPINNER true
+if get_option('SPINNER')
+    spinner_dep = [
+        dependency('cairo', required: true),
+        dependency('libdrm', required: true)
+    ]
+
+    spinner_src = [
+        'spinner.c',
+        'spinner_conf.c'
+    ]
+
+    args = ['-DSPINNER']
+
+    executable('spinner',
+        spinner_src,
+        dependencies: spinner_dep,
+        link_with: libplatsch,
+        c_args: args,
+        install: true,
+        include_directories: include_directories('.')
+    )
+endif
diff --git a/meson_options.txt b/meson_options.txt
new file mode 100644
index 0000000..d1149fe
--- /dev/null
+++ b/meson_options.txt
@@ -0,0 +1 @@
+option('SPINNER', type: 'boolean', value: false, description: 'Enable spinner build')
diff --git a/spinner.c b/spinner.c
new file mode 100644
index 0000000..19dbb8e
--- /dev/null
+++ b/spinner.c
@@ -0,0 +1,297 @@
+
+#include <cairo.h>
+#include <math.h>
+#include <sys/time.h>
+
+#include "libplatsch.h"
+#include "spinner_conf.h"
+
+typedef struct spinner {
+	cairo_format_t fmt;
+	cairo_surface_t *background_surface;
+	cairo_surface_t *symbol_surface;
+	cairo_surface_t *drawing_surface;
+	cairo_t *cr_background;
+	cairo_t *cr_drawing;
+	cairo_t *disp_cr;
+	int background_height;
+	int background_width;
+	int display_height;
+	int display_width;
+	int symbol_height;
+	int symbol_width;
+	struct modeset_dev *dev;
+	struct spinner *next;
+} spinner_t;
+
+void on_draw_Sequence_animation(cairo_t *cr, spinner_t *data)
+{
+	static int current_frame;
+	int num_frames = data->symbol_width / data->symbol_height;
+	int frame_width = data->symbol_height;
+
+	cairo_set_source_surface(cr, data->background_surface, 0, 0);
+	cairo_paint(cr);
+
+	cairo_save(cr);
+
+	cairo_translate(cr, data->display_width / 2, data->display_height / 2);
+
+	cairo_set_source_surface(cr, data->symbol_surface,
+				 -frame_width / 2 - current_frame * frame_width, -frame_width / 2);
+
+	cairo_rectangle(cr, -frame_width / 2, -frame_width / 2, frame_width, frame_width);
+	cairo_clip(cr);
+	cairo_paint(cr);
+
+	cairo_restore(cr);
+
+	current_frame = (current_frame + 1) % num_frames;
+}
+
+void on_draw_rotation_animation(cairo_t *cr, spinner_t *data)
+{
+	static float angle = 0.0;
+
+	cairo_set_source_surface(cr, data->background_surface, 0, 0);
+	cairo_paint(cr);
+	cairo_save(cr);
+	cairo_translate(cr, data->background_width / 2, data->background_height / 2);
+	cairo_rotate(cr, angle);
+	cairo_translate(cr, -data->symbol_width / 2, -data->symbol_height / 2);
+	cairo_set_source_surface(cr, data->symbol_surface, 0, 0);
+	cairo_paint(cr);
+	cairo_restore(cr);
+	angle += 0.1;
+	if (angle > 2 * M_PI)
+		angle = 0.0;
+}
+
+static uint32_t convert_to_cairo_format(uint32_t format)
+{
+	switch (format) {
+	case DRM_FORMAT_RGB565:
+		return CAIRO_FORMAT_RGB16_565;
+	case DRM_FORMAT_XRGB8888:
+		return CAIRO_FORMAT_ARGB32;
+	}
+	return CAIRO_FORMAT_INVALID;
+}
+
+static void cairo_draw_text(cairo_t *cr, Config config)
+{
+	if (strlen(config.text) > 0) {
+		cairo_select_font_face(cr, config.text_font, CAIRO_FONT_SLANT_NORMAL,
+				       CAIRO_FONT_WEIGHT_NORMAL);
+		cairo_set_font_size(cr, (double)config.text_size);
+		cairo_move_to(cr, config.text_x, config.text_y);
+		cairo_set_source_rgb(cr, 0, 0, 0);
+		cairo_show_text(cr, config.text);
+	}
+}
+
+static int cairo_update_display_surface(struct modeset_dev *dev, spinner_t *spinner_node)
+{
+	cairo_surface_t *disp_surface = cairo_image_surface_create_for_data(
+	    dev->map, convert_to_cairo_format(dev->format->format), dev->width, dev->height,
+	    dev->stride);
+	if (cairo_surface_status(disp_surface) != CAIRO_STATUS_SUCCESS) {
+		fprintf(stderr, "Failed to get cairo surface\n");
+		return EXIT_FAILURE;
+	}
+	spinner_node->display_width = cairo_image_surface_get_width(disp_surface);
+	spinner_node->display_height = cairo_image_surface_get_height(disp_surface);
+	spinner_node->fmt = cairo_image_surface_get_format(disp_surface);
+	spinner_node->disp_cr = cairo_create(disp_surface);
+	return EXIT_SUCCESS;
+}
+
+// create a surface and paint background image
+static int cairo_create_background_surface(spinner_t *spinner_node, Config config, const char *dir,
+					   const char *base)
+{
+	char filename[128];
+	int ret;
+
+	ret = snprintf(filename, sizeof(filename), "%s/%s.png", dir, base);
+	if (ret >= sizeof(filename)) {
+		error("Failed to fit filename into buffer\n");
+		return EXIT_FAILURE;
+	}
+
+	spinner_node->background_surface = cairo_image_surface_create(
+	    spinner_node->fmt, spinner_node->display_width, spinner_node->display_height);
+	if (cairo_surface_status(spinner_node->background_surface) != CAIRO_STATUS_SUCCESS) {
+		error("Failed to create background surface\n");
+		return EXIT_FAILURE;
+	}
+
+	cairo_surface_t *bk_img_surface = cairo_image_surface_create_from_png(filename);
+
+	if (cairo_surface_status(bk_img_surface) != CAIRO_STATUS_SUCCESS) {
+		error("Failed to create cairo surface from %s\n", filename);
+		return EXIT_FAILURE;
+	}
+
+	int image_width = cairo_image_surface_get_width(bk_img_surface);
+	int image_height = cairo_image_surface_get_height(bk_img_surface);
+	double scale_x = (double)spinner_node->display_width / image_width;
+	double scale_y = (double)spinner_node->display_height / image_height;
+
+	spinner_node->cr_background = cairo_create(spinner_node->background_surface);
+	cairo_scale(spinner_node->cr_background, scale_x, scale_y);
+	cairo_set_source_surface(spinner_node->cr_background, bk_img_surface, 0, 0);
+
+	spinner_node->background_width =
+	    cairo_image_surface_get_width(spinner_node->background_surface);
+	spinner_node->background_height =
+	    cairo_image_surface_get_height(spinner_node->background_surface);
+
+	cairo_paint(spinner_node->cr_background);
+	cairo_draw_text(spinner_node->cr_background, config);
+	return EXIT_SUCCESS;
+}
+
+static int cairo_create_symbol_surface(spinner_t *spinner_node, Config config)
+{
+	if (strlen(config.symbol) == 0)
+		return EXIT_FAILURE;
+
+	spinner_node->symbol_surface = cairo_image_surface_create_from_png(config.symbol);
+	if (cairo_surface_status(spinner_node->symbol_surface) != CAIRO_STATUS_SUCCESS) {
+		fprintf(stderr, "Failed loading %s\n", config.symbol);
+		spinner_node->symbol_surface = NULL;
+		return EXIT_FAILURE;
+	}
+	spinner_node->symbol_width = cairo_image_surface_get_width(spinner_node->symbol_surface);
+	spinner_node->symbol_height = cairo_image_surface_get_height(spinner_node->symbol_surface);
+	return EXIT_SUCCESS;
+}
+
+int main(int argc, char *argv[])
+{
+	bool pid1 = getpid() == 1;
+	char filename[128];
+	Config config = DEFAULT_CONFIG;
+	const char *base = "splash";
+	const char *dir = "/usr/share/platsch";
+	const char *env;
+	int frames;
+	int ret;
+	long elapsed_time;
+
+	spinner_t *spinner_list = NULL, *spinner_node = NULL, *spinner_iter = NULL;
+	struct modeset_dev *iter;
+	struct timeval start, end;
+
+	env = getenv("platsch_directory");
+	if (env)
+		dir = env;
+
+	env = getenv("platsch_basename");
+	if (env)
+		base = env;
+
+	ret = snprintf(filename, sizeof(filename), "%s/spinner.conf", dir);
+	if (ret >= sizeof(filename)) {
+		error("Failed to fit filename\n");
+		return EXIT_FAILURE;
+	}
+
+	parseConfig(filename, &config);
+
+	struct modeset_dev *modeset_list = init();
+
+	if (!modeset_list) {
+		fprintf(stderr, "Failed to initialize modeset\n");
+		return EXIT_FAILURE;
+	}
+
+	for (iter = modeset_list; iter; iter = iter->next) {
+		spinner_node = (spinner_t *)malloc(sizeof(spinner_t));
+		if (!spinner_node) {
+			fprintf(stderr, "Failed to allocate memory for spinner_node\n");
+			return EXIT_FAILURE;
+		}
+		memset(spinner_node, 0, sizeof(*spinner_node));
+
+		if (cairo_update_display_surface(iter, spinner_node))
+			return EXIT_FAILURE;
+
+		if (cairo_create_background_surface(spinner_node, config, dir, base))
+			return EXIT_FAILURE;
+
+		cairo_create_symbol_surface(spinner_node, config);
+
+		spinner_node->drawing_surface = cairo_image_surface_create(
+		    spinner_node->fmt, spinner_node->display_width, spinner_node->display_height);
+		if (cairo_surface_status(spinner_node->drawing_surface) != CAIRO_STATUS_SUCCESS) {
+			error("Failed to create drawing surface\n");
+			return EXIT_FAILURE;
+		}
+		// create cairo context for drawing surface to avoid frlickering
+		spinner_node->cr_drawing = cairo_create(spinner_node->drawing_surface);
+		// draw static image with text as first frame
+		cairo_set_source_surface(spinner_node->disp_cr, spinner_node->background_surface, 0,
+					 0);
+		cairo_paint(spinner_node->disp_cr);
+		update_display(iter);
+
+		spinner_node->next = spinner_list;
+		spinner_list = spinner_node;
+	}
+
+	if (pid1) {
+		char **initsargv;
+
+		ret = fork();
+		printf("fork ret=%d\n", ret);
+		if (ret < 0)
+			error("failed to fork for init: %m\n");
+		else if (ret == 0)
+			goto drawing;
+
+		initsargv = calloc(sizeof(argv[0]), argc + 1);
+
+		if (!initsargv) {
+			error("failed to allocate argv for init\n");
+			return EXIT_FAILURE;
+		}
+		memcpy(initsargv, argv, argc * sizeof(argv[0]));
+		initsargv[0] = "/sbin/init";
+		initsargv[argc] = NULL;
+
+		execv("/sbin/init", initsargv);
+
+		error("failed to exec init: %m\n");
+
+		return EXIT_FAILURE;
+	}
+
+drawing:
+	frames = config.frames;
+	while (config.frames == 0 || frames--) {
+		gettimeofday(&start, NULL);
+		for (spinner_iter = spinner_list; spinner_iter; spinner_iter = spinner_iter->next) {
+			if (spinner_node->symbol_surface == NULL)
+				usleep(1000000 / config.fps);
+			if (spinner_node->symbol_width / spinner_node->symbol_height > 2)
+				on_draw_Sequence_animation(spinner_iter->cr_drawing, spinner_iter);
+			else
+				on_draw_rotation_animation(spinner_iter->cr_drawing, spinner_iter);
+
+			cairo_set_source_surface(spinner_iter->disp_cr,
+						 spinner_iter->drawing_surface, 0, 0);
+			cairo_paint(spinner_iter->disp_cr);
+		}
+		gettimeofday(&end, NULL);
+		elapsed_time =
+		    (end.tv_sec - start.tv_sec) * 1000000 + (end.tv_usec - start.tv_usec);
+
+		long sleep_time = (1000000 / config.fps) - elapsed_time;
+
+		if (sleep_time > 0)
+			usleep(sleep_time);
+	}
+	redirect_stdfd();
+}
diff --git a/spinner_conf.c b/spinner_conf.c
new file mode 100644
index 0000000..e1f6943
--- /dev/null
+++ b/spinner_conf.c
@@ -0,0 +1,69 @@
+#include "spinner_conf.h"
+#include <errno.h>
+
+int parseConfig(const char *filename, Config *config)
+{
+	char *value_end;
+	char *value_start;
+	char key[MAX_LINE_LENGTH];
+	char line[MAX_LINE_LENGTH];
+	char value[MAX_LINE_LENGTH];
+	FILE *file;
+
+	file = fopen(filename, "r");
+	if (file == NULL) {
+		fprintf(stderr, "Unable to open file: %s\n", filename);
+		return -EFAULT;
+	}
+
+	while (fgets(line, sizeof(line), file)) {
+		if (strlen(line) > MAX_LINE_LENGTH) {
+			fprintf(stderr, "conf string too long\n");
+			continue;
+		}
+		if (line[0] != '#' && sscanf(line, "%[^=]=%[^\n]", key, value) == 2) {
+			value_start = strchr(line, '=') + 1;
+			value_end = line + strlen(line) - 1;
+
+			while (isspace(*value_start))
+				value_start++;
+			while (isspace(*value_end) || *value_end == '"')
+				value_end--;
+
+			if (*value_start == '"')
+				value_start++;
+			if (*value_end == '"')
+					value_end--;
+
+			if (value_end < value_start) {
+				memset(value, 0, sizeof(value));
+				continue;
+			} else {
+				strncpy(value, value_start, value_end - value_start + 1);
+				value[value_end - value_start + 1] = '\0';
+				value[sizeof(value) - 1] = '\0';
+			}
+			value[MAX_LINE_LENGTH - 1] = '\0';
+
+			if (strcmp(key, "symbol") == 0) {
+				strncpy(config->symbol, value, MAX_LINE_LENGTH);
+			} else if (strcmp(key, "text") == 0) {
+				strncpy(config->text, value, MAX_LINE_LENGTH);
+			} else if (strcmp(key, "fps") == 0) {
+				config->fps = atoi(value);
+			} else if (strcmp(key, "frames") == 0) {
+				config->frames = atoi(value);
+			} else if (strcmp(key, "text_x") == 0) {
+				config->text_x = atoi(value);
+			} else if (strcmp(key, "text_y") == 0) {
+				config->text_y = atoi(value);
+			} else if (strcmp(key, "text_font") == 0) {
+				strncpy(config->text_font, value, MAX_LINE_LENGTH);
+			} else if (strcmp(key, "text_size") == 0) {
+				config->text_size = atoi(value);
+			}
+		}
+	}
+	fclose(file);
+	return 0;
+}
diff --git a/spinner_conf.h b/spinner_conf.h
new file mode 100644
index 0000000..a9491c8
--- /dev/null
+++ b/spinner_conf.h
@@ -0,0 +1,35 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <ctype.h>
+
+#ifndef __SPINNER_CONF_H__
+#define __SPINNER_CONF_H__
+
+
+#define MAX_LINE_LENGTH 128
+
+typedef struct {
+	char symbol[MAX_LINE_LENGTH];
+	int fps;
+	int frames;
+	int text_x;
+	int text_y;
+	char text_font[MAX_LINE_LENGTH];
+	int text_size;
+	char text[MAX_LINE_LENGTH];
+} Config;
+
+int parseConfig(const char *filename, Config *config);
+
+#define DEFAULT_CONFIG { \
+	.symbol = "", \
+	.fps = 20, \
+	.frames = 0, \
+	.text_x = 0, \
+	.text_y = 0, \
+	.text_font = "Sans", \
+	.text_size = 30, \
+	.text = "" \
+}
+#endif
-- 
2.34.1




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

* Re: [OSS-Tools] [PATCH platsch V3 2/4] convert to meson build
  2024-06-13  7:07 ` [OSS-Tools] [PATCH platsch V3 2/4] convert to meson build LI Qingwu
@ 2024-06-13  8:44   ` Michael Tretter
  2024-06-14  5:52   ` Ulrich Ölmann
  1 sibling, 0 replies; 9+ messages in thread
From: Michael Tretter @ 2024-06-13  8:44 UTC (permalink / raw)
  To: LI Qingwu; +Cc: oss-tools, m.felsch

Hi,

On Thu, 13 Jun 2024 09:07:22 +0200, LI Qingwu wrote:
> Convert to meson build and update the README.rst
> 
> Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> ---
>  Makefile.am  | 23 -----------------------
>  README.rst   |  8 ++++++++
>  configure.ac | 13 -------------
>  meson.build  | 15 +++++++++++++++
>  4 files changed, 23 insertions(+), 36 deletions(-)
>  delete mode 100644 Makefile.am
>  delete mode 100644 configure.ac
>  create mode 100644 meson.build
> 
> diff --git a/Makefile.am b/Makefile.am
> deleted file mode 100644
> index d149ae0..0000000
> --- a/Makefile.am
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -EXTRA_DIST = README.rst LICENSE
> -
> -sbin_PROGRAMS = platsch
> -
> -platsch_SOURCES = platsch.c
> -platsch_CFLAGS = $(LIBDRM_CFLAGS)
> -platsch_LDADD = $(LIBDRM_LIBS)
> -
> -CLEANFILES = \
> -        $(DIST_ARCHIVES)
> -
> -DISTCLEAN = \
> -        config.log \
> -        config.status \
> -        Makefile
> -
> -MAINTAINERCLEANFILES = \
> -	aclocal.m4 \
> -	configure \
> -	depcomp \
> -	install-sh \
> -	Makefile.in \
> -	missing
> diff --git a/README.rst b/README.rst
> index e318120..f1c0812 100644
> --- a/README.rst
> +++ b/README.rst
> @@ -141,3 +141,11 @@ By adding a Signed-off-by line (e.g. using ``git commit -s``) saying::
>  
>  (using your real name and e-mail address), you state that your contributions
>  are in line with the DCO.
> +
> +Compiling Instructions
> +----------------------------
> +
> +.. code-block:: shell
> +
> +    meson setup build
> +    meson compile -C build
> diff --git a/configure.ac b/configure.ac
> deleted file mode 100644
> index 18878db..0000000
> --- a/configure.ac
> +++ /dev/null
> @@ -1,13 +0,0 @@
> -AC_PREREQ([2.69])
> -AC_INIT([platsch], [2019.12.0], [oss-tools@pengutronix.de])
> -AC_CONFIG_SRCDIR([platsch.c])
> -AM_INIT_AUTOMAKE([foreign dist-xz])
> -
> -AC_PROG_CC
> -AC_PROG_MAKE_SET
> -
> -PKG_CHECK_MODULES([LIBDRM], [libdrm >= 2.4.112])
> -
> -AC_CONFIG_FILES([Makefile])
> -
> -AC_OUTPUT
> diff --git a/meson.build b/meson.build
> new file mode 100644
> index 0000000..9f6be1e
> --- /dev/null
> +++ b/meson.build
> @@ -0,0 +1,15 @@
> +project('platsch', 'c')
> +
> +platsch_dep = dependency('libdrm', version : '>= 2.4.112', required : true)

The variable name is a bit unusual. Usually, the name of the library in
the dependency is used in the variable name.

I'd suggest calling it dep_libdrm.

Michael

> +sources = ['platsch.c']
> +
> +# Define the headers
> +headers = ['platsch.h']
> +
> +# Create the platsch executable
> +executable('platsch',
> +    sources,
> +    dependencies: platsch_dep,
> +    install: true,
> +    include_directories: include_directories('.')
> +)
> -- 
> 2.34.1
> 
> 
> 



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

* Re: [OSS-Tools] [PATCH platsch V3 1/4] platsch: constify draw_buffer
  2024-06-13  7:07 [OSS-Tools] [PATCH platsch V3 1/4] platsch: constify draw_buffer LI Qingwu
                   ` (2 preceding siblings ...)
  2024-06-13  7:07 ` [OSS-Tools] [PATCH platsch V3 4/4] Add spinner executable for boot animation and text show LI Qingwu
@ 2024-06-14  5:30 ` Ulrich Ölmann
  3 siblings, 0 replies; 9+ messages in thread
From: Ulrich Ölmann @ 2024-06-14  5:30 UTC (permalink / raw)
  To: LI Qingwu; +Cc: oss-tools, m.felsch

Hi Qingwu Li,

On Thu, Jun 13 2024 at 09:07 +0200, LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn> wrote:
> From: Marco Felsch <m.felsch@pengutronix.de>
>
> Make the dir and base const.
>
> Upstream-Status: Pending

please drop the last line, it is only required in the context of a
Yocto-BSP.

Best regards
Ulrich


> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  platsch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/platsch.c b/platsch.c
> index 535b589..1aaa8d5 100644
> --- a/platsch.c
> +++ b/platsch.c
> @@ -111,7 +111,7 @@ struct modeset_dev {
>  	uint32_t crtc_id;
>  };
>  
> -void draw_buffer(struct modeset_dev *dev, char *dir, char *base)
> +void draw_buffer(struct modeset_dev *dev, const char *dir, const char *base)
>  {
>  	int fd_src;
>  	char filename[128];
-- 
Pengutronix e.K.                           | Ulrich Ölmann               |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



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

* Re: [OSS-Tools] [PATCH platsch V3 2/4] convert to meson build
  2024-06-13  7:07 ` [OSS-Tools] [PATCH platsch V3 2/4] convert to meson build LI Qingwu
  2024-06-13  8:44   ` Michael Tretter
@ 2024-06-14  5:52   ` Ulrich Ölmann
  1 sibling, 0 replies; 9+ messages in thread
From: Ulrich Ölmann @ 2024-06-14  5:52 UTC (permalink / raw)
  To: LI Qingwu; +Cc: oss-tools, m.felsch

Hi Qing-Wu Li,

On Thu, Jun 13 2024 at 09:07 +0200, LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn> wrote:
> Convert to meson build and update the README.rst
>
> Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> ---
>  Makefile.am  | 23 -----------------------
>  README.rst   |  8 ++++++++
>  configure.ac | 13 -------------
>  meson.build  | 15 +++++++++++++++
>  4 files changed, 23 insertions(+), 36 deletions(-)
>  delete mode 100644 Makefile.am
>  delete mode 100644 configure.ac
>  create mode 100644 meson.build
>
> diff --git a/Makefile.am b/Makefile.am
> deleted file mode 100644
> index d149ae0..0000000
> --- a/Makefile.am
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -EXTRA_DIST = README.rst LICENSE
> -
> -sbin_PROGRAMS = platsch
> -
> -platsch_SOURCES = platsch.c
> -platsch_CFLAGS = $(LIBDRM_CFLAGS)
> -platsch_LDADD = $(LIBDRM_LIBS)
> -
> -CLEANFILES = \
> -        $(DIST_ARCHIVES)
> -
> -DISTCLEAN = \
> -        config.log \
> -        config.status \
> -        Makefile
> -
> -MAINTAINERCLEANFILES = \
> -	aclocal.m4 \
> -	configure \
> -	depcomp \
> -	install-sh \
> -	Makefile.in \
> -	missing
> diff --git a/README.rst b/README.rst
> index e318120..f1c0812 100644
> --- a/README.rst
> +++ b/README.rst
> @@ -141,3 +141,11 @@ By adding a Signed-off-by line (e.g. using ``git commit -s``) saying::
>  
>  (using your real name and e-mail address), you state that your contributions
>  are in line with the DCO.
> +
> +Compiling Instructions
> +----------------------------
> +
> +.. code-block:: shell
> +
> +    meson setup build
> +    meson compile -C build
> diff --git a/configure.ac b/configure.ac
> deleted file mode 100644
> index 18878db..0000000
> --- a/configure.ac
> +++ /dev/null
> @@ -1,13 +0,0 @@
> -AC_PREREQ([2.69])
> -AC_INIT([platsch], [2019.12.0], [oss-tools@pengutronix.de])

Uwe Kleine-König defined platsch's version here to be 2019.12.0 in the
project's very first commit. Please take the chance to update it to
2024.06.0 when switching to meson...

> -AC_CONFIG_SRCDIR([platsch.c])
> -AM_INIT_AUTOMAKE([foreign dist-xz])
> -
> -AC_PROG_CC
> -AC_PROG_MAKE_SET
> -
> -PKG_CHECK_MODULES([LIBDRM], [libdrm >= 2.4.112])
> -
> -AC_CONFIG_FILES([Makefile])
> -
> -AC_OUTPUT
> diff --git a/meson.build b/meson.build
> new file mode 100644
> index 0000000..9f6be1e
> --- /dev/null
> +++ b/meson.build
> @@ -0,0 +1,15 @@
> +project('platsch', 'c')

by using

  project('platsch', 'c', version: '2024.06.0')

instead (although it is not used anywhere in the code up to now which
probably should be changed in the future).

Best regards
Ulrich


> +
> +platsch_dep = dependency('libdrm', version : '>= 2.4.112', required : true)
> +sources = ['platsch.c']
> +
> +# Define the headers
> +headers = ['platsch.h']
> +
> +# Create the platsch executable
> +executable('platsch',
> +    sources,
> +    dependencies: platsch_dep,
> +    install: true,
> +    include_directories: include_directories('.')
> +)
-- 
Pengutronix e.K.                           | Ulrich Ölmann               |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



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

* Re: [OSS-Tools] [PATCH platsch V3 3/4] platsch: split into platsch and libplatsch
  2024-06-13  7:07 ` [OSS-Tools] [PATCH platsch V3 3/4] platsch: split into platsch and libplatsch LI Qingwu
@ 2024-06-14 10:57   ` Ulrich Ölmann
  2024-06-17  6:44     ` LI Qingwu
  0 siblings, 1 reply; 9+ messages in thread
From: Ulrich Ölmann @ 2024-06-14 10:57 UTC (permalink / raw)
  To: LI Qingwu; +Cc: oss-tools, m.felsch

Hi Qing-Wu Li,

On Thu, Jun 13 2024 at 09:07 +0200, LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn> wrote:
> Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> ---
>  libplatsch.c | 590 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  libplatsch.h |  51 +++++
>  meson.build  |  13 +-
>  platsch.c    | 582 +-------------------------------------------------
>  4 files changed, 658 insertions(+), 578 deletions(-)
>  create mode 100644 libplatsch.c
>  create mode 100644 libplatsch.h
>
> diff --git a/libplatsch.c b/libplatsch.c
> new file mode 100644
> index 0000000..1d48e0e
> --- /dev/null
> +++ b/libplatsch.c
> @@ -0,0 +1,590 @@
> +/*
> + * Copyright (C) 2019 Pengutronix, Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> + *
> + * Permission to use, copy, modify, and/or distribute this software
> + * for any purpose with or without fee is hereby granted.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
> + * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
> + * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE
> + * AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
> + * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
> + * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
> + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + *
> + * Some code parts base on example code written in 2012 by David Herrmann
> + * <dh.herrmann@googlemail.com> and dedicated to the Public Domain. It was found
> + * in 2019 on
> + * https://raw.githubusercontent.com/dvdhrm/docs/master/drm-howto/modeset.c
> + */
> +
> +#include <assert.h>
> +#include <ctype.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <getopt.h>
> +#include <libgen.h>
> +#include <stdarg.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/mman.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <unistd.h>

if there is no requirement for a new order of inclusion please keep the
sorting found in platsch.c.

> +#include "libplatsch.h"
> +#include <drm_fourcc.h>
> +#include <xf86drm.h>
> +#include <xf86drmMode.h>

The includes xf86drmMode.h and drm_fourcc.h are already included in
libplatsch.h, so there is probably no need to include them again here.

> +#include "libplatsch.h"

This was already done some lines above.

> +void redirect_stdfd(void)
> +{
> +	int devnull = open("/dev/null", O_RDWR, 0);
> +
> +	if (devnull < 0) {
> +		error("Failed to open /dev/null: %m\n");
> +		return;
> +	}
> +
> +	close(STDIN_FILENO);
> +	dup2(devnull, STDIN_FILENO);
> +	close(STDOUT_FILENO);
> +	dup2(devnull, STDOUT_FILENO);
> +	close(STDERR_FILENO);
> +	dup2(devnull, STDERR_FILENO);
> +	close(devnull);
> +}

[...]

> +static int drmfd;
> +
> +struct modeset_dev *init(void)
> +{
> +	static char drmdev[128];

This hasn't been static before, is that really needed?

> +	int ret = 0, i;
> +
> +	for (i = 0; i < 64; i++) {
> +		struct drm_mode_card_res res = {0};
> +
> +		/*
> +		 * XXX: Maybe use drmOpen instead?
> +		 * (Where should name/busid come from?)
> +		 * XXX: Loop through drm devices to find one with connectors.
> +		 */
> +		ret = snprintf(drmdev, sizeof(drmdev), DRM_DEV_NAME, DRM_DIR_NAME, i);
> +		if (ret >= sizeof(drmdev)) {
> +			error("Huh, device name overflowed buffer\n");
> +			goto execinit;
> +		}
> +
> +		drmfd = open(drmdev, O_RDWR | O_CLOEXEC, 0);
> +		if (drmfd < 0) {
> +			error("Failed to open drm device: %m\n");
> +			goto execinit;
> +		}
> +
> +		ret = drmIoctl(drmfd, DRM_IOCTL_MODE_GETRESOURCES, &res);
> +		if (ret < 0) {
> +			close(drmfd);
> +			continue;
> +		} else {
> +			/* Device found */
> +			break;
> +		}
> +	}
> +
> +	if (i == 64) {
> +		error("No suitable DRM device found\n");
> +		goto execinit;
> +	}
> +
> +	ret = drmprepare(drmfd);
> +	if (ret) {
> +		error("Failed to prepare DRM device\n");
> +		goto execinit;
> +	}
> +
> +	return modeset_list;
> +
> +execinit:

The label should probably be renamed as this new init()-function has
nothing to do with calling exec() for the next PID1 program anymore.

> +	return NULL;
> +}
> +
> +void update_display(struct modeset_dev *dev)
> +{
> +	int ret = 0;
> +
> +	if (dev->setmode) {
> +		ret = drmModeSetCrtc(drmfd, dev->crtc_id, dev->fb_id, 0, 0, &dev->conn_id, 1,
> +				     &dev->mode);
> +		if (ret) {
> +			error("Cannot set CRTC for connector #%u: %m\n", dev->conn_id);
> +		}
> +		dev->setmode = 0;

This line is new. If it is needed, please add it in a separate commit
and describe why it is needed.

> +	} else {
> +		ret = drmModePageFlip(drmfd, dev->crtc_id, dev->fb_id, 0, NULL);
> +		if (ret) {
> +			error("Page flip failed on connector #%u: %m\n", dev->conn_id);
> +		}
> +	}
> +}
> +
> +void draw(struct modeset_dev *dev, const char *dir, const char *base)
> +{
> +	draw_buffer(dev, dir, base);
> +	update_display(dev);
> +}
> +
> +void finish(void) { drmDropMaster(drmfd); }

The original code checked the return value here. Is drmDropMaster()
guaranteed to always succeed?

> +void deinit(void)
> +{
> +	struct modeset_dev *iter;
> +
> +	for (iter = modeset_list; iter; iter = iter->next) {

Use after free: you are accessing iter->next here...

> +		if (iter->map) {
> +			munmap(iter->map, iter->size);
> +		}
> +		if (iter->fb_id) {
> +			drmModeRmFB(drmfd, iter->fb_id);
> +		}
> +		free(iter);

... although iter was already freed here.

> +	}
> +}
> diff --git a/libplatsch.h b/libplatsch.h
> new file mode 100644
> index 0000000..77f74a9
> --- /dev/null
> +++ b/libplatsch.h
> @@ -0,0 +1,51 @@
> +#ifndef __LIBPLATSCH_H__
> +#define __LIBPLATSCH_H__
> +
> +#include <xf86drmMode.h>
> +#include <drm_fourcc.h>
> +#include "libplatsch.h"

Probably no need to include libplatsch.h from itself.

> +#include <unistd.h>
> +
> +
> +
> +#define debug(fmt, ...) printf("%s:%d: " fmt, __func__, __LINE__, ##__VA_ARGS__)
> +#define error(fmt, ...) fprintf(stderr, fmt, ##__VA_ARGS__)
> +
> +#define ARRAY_SIZE(a) (sizeof(a) / sizeof(*a))
> +
> +struct platsch_format {
> +	uint32_t format;
> +	uint32_t bpp;
> +	const char *name;
> +};
> +
> +struct modeset_dev {
> +	struct modeset_dev *next;
> +
> +	uint32_t width;
> +	uint32_t height;
> +	uint32_t stride;
> +	uint32_t size;
> +	const struct platsch_format *format;
> +	uint32_t handle;
> +	void *map;
> +	bool setmode;
> +	drmModeModeInfo mode;
> +	uint32_t fb_id;
> +	uint32_t conn_id;
> +	uint32_t crtc_id;
> +};
> +
> +static const struct platsch_format platsch_formats[] = {
> +	{ DRM_FORMAT_RGB565, 16, "RGB565" }, /* default */
> +	{ DRM_FORMAT_XRGB8888, 32, "XRGB8888" },
> +};
> +
> +ssize_t readfull(int fd, void *buf, size_t count);
> +struct modeset_dev *init(void);
> +void draw(struct modeset_dev *dev, const char *dir, const char *base);
> +void finish(void);
> +void redirect_stdfd(void);
> +void update_display(struct modeset_dev *dev);

Only the minority of symbols in the new library are namespaced, compare
"platsch_format" against "draw" or "finish". Particularly the latter can
be expected to be exported from third party code which libplatsch is
then not able to be linked against. Thus please add the prefix
"platsch_" to at least all symbols that are published via libplatsch.h.

> +#endif
> \ No newline at end of file
> diff --git a/meson.build b/meson.build
> index 9f6be1e..dc55238 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1,15 +1,18 @@
>  project('platsch', 'c')
>  
>  platsch_dep = dependency('libdrm', version : '>= 2.4.112', required : true)

One feature of platsch is to start up fast, hence it is intentionally
linked against static libraries only. Therefore please use

  platsch_dep = dependency('libdrm',
      version: '>= 2.4.112',
      static: true,
      required: true
  )

instead.

> -sources = ['platsch.c']
> +sources = ['libplatsch.c']
>  
> -# Define the headers
> -headers = ['platsch.h']
>  
> -# Create the platsch executable
> -executable('platsch',
> +libplatsch = static_library('libplatsch',

The resulting library is called "liblibplatsch.a" now which is somewhat
redundant.

>      sources,
>      dependencies: platsch_dep,
> +)
> +
> +executable('platsch',
> +    'platsch.c',
> +    dependencies: platsch_dep,
> +    link_with: libplatsch,

Unfortunately picking the static variant of libdrm in its dependency()-
statement is not enough to get a statically linked executable "platsch"
as there are still some dynamically linked objects left, e.g. the
c-library. Thus please add

    link_args: '-static',

to the executable() statement.

Best regards
Ulrich


>      install: true,
>      include_directories: include_directories('.')
>  )

[...]

> -execinit:
>  	if (pid1) {
>  		ret = fork();
>  		if (ret < 0) {
-- 
Pengutronix e.K.                           | Ulrich Ölmann               |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



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

* Re: [OSS-Tools] [PATCH platsch V3 3/4] platsch: split into platsch and libplatsch
  2024-06-14 10:57   ` Ulrich Ölmann
@ 2024-06-17  6:44     ` LI Qingwu
  0 siblings, 0 replies; 9+ messages in thread
From: LI Qingwu @ 2024-06-17  6:44 UTC (permalink / raw)
  To: Ulrich Ölmann; +Cc: oss-tools, m.felsch

Hi Ulrich,

Thanks your input, I will address in coming version.
Just question about static link as bellow.

> -----Original Message-----
> From: Ulrich Ölmann <u.oelmann@pengutronix.de>
> Sent: Friday, June 14, 2024 6:57 PM
> To: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> Cc: oss-tools@pengutronix.de; m.felsch@pengutronix.de
> Subject: Re: [OSS-Tools] [PATCH platsch V3 3/4] platsch: split into platsch and
> libplatsch
>
> [你通常不会收到来自 u.oelmann@pengutronix.de 的电子邮件。请访问
> https://aka.ms/LearnAboutSenderIdentification,以了解这一点为什么很重要]
>
> This email is not from Hexagon’s Office 365 instance. Please be careful while
> clicking links, opening attachments, or replying to this email.
>
>
> Hi Qing-Wu Li,
>
> On Thu, Jun 13 2024 at 09:07 +0200, LI Qingwu
> <Qing-wu.Li@leica-geosystems.com.cn> wrote:
> > Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> > ---
> >  libplatsch.c | 590
> > +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  libplatsch.h |  51 +++++
> >  meson.build  |  13 +-
> >  platsch.c    | 582 +-------------------------------------------------
> >  4 files changed, 658 insertions(+), 578 deletions(-)  create mode
> > 100644 libplatsch.c  create mode 100644 libplatsch.h
> >
> > diff --git a/libplatsch.c b/libplatsch.c new file mode 100644 index
> > 0000000..1d48e0e
> > --- /dev/null
> > +++ b/libplatsch.c
> > @@ -0,0 +1,590 @@
> > +/*
> > + * Copyright (C) 2019 Pengutronix, Uwe Kleine-König
> > +<u.kleine-koenig@pengutronix.de>
> > + *
> > + * Permission to use, copy, modify, and/or distribute this software
> > + * for any purpose with or without fee is hereby granted.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
> > + * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
> IMPLIED
> > + * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
> THE
> > + * AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
> > + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
> FROM
> > +LOSS
> > + * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
> > + * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
> > + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> > + *
> > + * Some code parts base on example code written in 2012 by David
> > +Herrmann
> > + * <dh.herrmann@googlemail.com> and dedicated to the Public Domain.
> > +It was found
> > + * in 2019 on
> > + *
> > +https://raw/
> >
> +.githubusercontent.com%2Fdvdhrm%2Fdocs%2Fmaster%2Fdrm-howto%2Fmo
> deset
> >
> +.c&data=05%7C02%7Cqing-wu.li%40leica-geosystems.com.cn%7C8fc4e5421e
> 3d
> >
> +42d7ffef08dc8c60bec5%7C1b16ab3eb8f64fe39f3e2db7fe549f6a%7C0%7C0%7
> C638
> >
> +539594360927380%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiL
> CJQIjoiV
> >
> +2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=5xytXSa
> tTpw4
> > +KV3tRtn9pb%2F%2Fos%2BCbE6dWY%2BbZ95VGd0%3D&reserved=0
> > + */
> > +
> > +#include <assert.h>
> > +#include <ctype.h>
> > +#include <errno.h>
> > +#include <fcntl.h>
> > +#include <getopt.h>
> > +#include <libgen.h>
> > +#include <stdarg.h>
> > +#include <stdbool.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <sys/mman.h>
> > +#include <sys/stat.h>
> > +#include <sys/types.h>
> > +#include <unistd.h>
>
> if there is no requirement for a new order of inclusion please keep the sorting
> found in platsch.c.
>
> > +#include "libplatsch.h"
> > +#include <drm_fourcc.h>
> > +#include <xf86drm.h>
> > +#include <xf86drmMode.h>
>
> The includes xf86drmMode.h and drm_fourcc.h are already included in
> libplatsch.h, so there is probably no need to include them again here.
>
> > +#include "libplatsch.h"
>
> This was already done some lines above.
>
> > +void redirect_stdfd(void)
> > +{
> > +     int devnull = open("/dev/null", O_RDWR, 0);
> > +
> > +     if (devnull < 0) {
> > +             error("Failed to open /dev/null: %m\n");
> > +             return;
> > +     }
> > +
> > +     close(STDIN_FILENO);
> > +     dup2(devnull, STDIN_FILENO);
> > +     close(STDOUT_FILENO);
> > +     dup2(devnull, STDOUT_FILENO);
> > +     close(STDERR_FILENO);
> > +     dup2(devnull, STDERR_FILENO);
> > +     close(devnull);
> > +}
>
> [...]
>
> > +static int drmfd;
> > +
> > +struct modeset_dev *init(void)
> > +{
> > +     static char drmdev[128];
>
> This hasn't been static before, is that really needed?
>
> > +     int ret = 0, i;
> > +
> > +     for (i = 0; i < 64; i++) {
> > +             struct drm_mode_card_res res = {0};
> > +
> > +             /*
> > +              * XXX: Maybe use drmOpen instead?
> > +              * (Where should name/busid come from?)
> > +              * XXX: Loop through drm devices to find one with
> connectors.
> > +              */
> > +             ret = snprintf(drmdev, sizeof(drmdev), DRM_DEV_NAME,
> DRM_DIR_NAME, i);
> > +             if (ret >= sizeof(drmdev)) {
> > +                     error("Huh, device name overflowed buffer\n");
> > +                     goto execinit;
> > +             }
> > +
> > +             drmfd = open(drmdev, O_RDWR | O_CLOEXEC, 0);
> > +             if (drmfd < 0) {
> > +                     error("Failed to open drm device: %m\n");
> > +                     goto execinit;
> > +             }
> > +
> > +             ret = drmIoctl(drmfd, DRM_IOCTL_MODE_GETRESOURCES,
> &res);
> > +             if (ret < 0) {
> > +                     close(drmfd);
> > +                     continue;
> > +             } else {
> > +                     /* Device found */
> > +                     break;
> > +             }
> > +     }
> > +
> > +     if (i == 64) {
> > +             error("No suitable DRM device found\n");
> > +             goto execinit;
> > +     }
> > +
> > +     ret = drmprepare(drmfd);
> > +     if (ret) {
> > +             error("Failed to prepare DRM device\n");
> > +             goto execinit;
> > +     }
> > +
> > +     return modeset_list;
> > +
> > +execinit:
>
> The label should probably be renamed as this new init()-function has nothing to
> do with calling exec() for the next PID1 program anymore.
>
> > +     return NULL;
> > +}
> > +
> > +void update_display(struct modeset_dev *dev) {
> > +     int ret = 0;
> > +
> > +     if (dev->setmode) {
> > +             ret = drmModeSetCrtc(drmfd, dev->crtc_id, dev->fb_id, 0, 0,
> &dev->conn_id, 1,
> > +                                  &dev->mode);
> > +             if (ret) {
> > +                     error("Cannot set CRTC for connector #%u: %m\n",
> dev->conn_id);
> > +             }
> > +             dev->setmode = 0;
>
> This line is new. If it is needed, please add it in a separate commit and describe
> why it is needed.
>
> > +     } else {
> > +             ret = drmModePageFlip(drmfd, dev->crtc_id, dev->fb_id, 0,
> NULL);
> > +             if (ret) {
> > +                     error("Page flip failed on connector #%u: %m\n",
> dev->conn_id);
> > +             }
> > +     }
> > +}
> > +
> > +void draw(struct modeset_dev *dev, const char *dir, const char *base)
> > +{
> > +     draw_buffer(dev, dir, base);
> > +     update_display(dev);
> > +}
> > +
> > +void finish(void) { drmDropMaster(drmfd); }
>
> The original code checked the return value here. Is drmDropMaster()
> guaranteed to always succeed?
>
> > +void deinit(void)
> > +{
> > +     struct modeset_dev *iter;
> > +
> > +     for (iter = modeset_list; iter; iter = iter->next) {
>
> Use after free: you are accessing iter->next here...
>
> > +             if (iter->map) {
> > +                     munmap(iter->map, iter->size);
> > +             }
> > +             if (iter->fb_id) {
> > +                     drmModeRmFB(drmfd, iter->fb_id);
> > +             }
> > +             free(iter);
>
> ... although iter was already freed here.
>
> > +     }
> > +}
> > diff --git a/libplatsch.h b/libplatsch.h new file mode 100644 index
> > 0000000..77f74a9
> > --- /dev/null
> > +++ b/libplatsch.h
> > @@ -0,0 +1,51 @@
> > +#ifndef __LIBPLATSCH_H__
> > +#define __LIBPLATSCH_H__
> > +
> > +#include <xf86drmMode.h>
> > +#include <drm_fourcc.h>
> > +#include "libplatsch.h"
>
> Probably no need to include libplatsch.h from itself.
>
> > +#include <unistd.h>
> > +
> > +
> > +
> > +#define debug(fmt, ...) printf("%s:%d: " fmt, __func__, __LINE__,
> > +##__VA_ARGS__) #define error(fmt, ...) fprintf(stderr, fmt,
> > +##__VA_ARGS__)
> > +
> > +#define ARRAY_SIZE(a) (sizeof(a) / sizeof(*a))
> > +
> > +struct platsch_format {
> > +     uint32_t format;
> > +     uint32_t bpp;
> > +     const char *name;
> > +};
> > +
> > +struct modeset_dev {
> > +     struct modeset_dev *next;
> > +
> > +     uint32_t width;
> > +     uint32_t height;
> > +     uint32_t stride;
> > +     uint32_t size;
> > +     const struct platsch_format *format;
> > +     uint32_t handle;
> > +     void *map;
> > +     bool setmode;
> > +     drmModeModeInfo mode;
> > +     uint32_t fb_id;
> > +     uint32_t conn_id;
> > +     uint32_t crtc_id;
> > +};
> > +
> > +static const struct platsch_format platsch_formats[] = {
> > +     { DRM_FORMAT_RGB565, 16, "RGB565" }, /* default */
> > +     { DRM_FORMAT_XRGB8888, 32, "XRGB8888" }, };
> > +
> > +ssize_t readfull(int fd, void *buf, size_t count); struct modeset_dev
> > +*init(void); void draw(struct modeset_dev *dev, const char *dir,
> > +const char *base); void finish(void); void redirect_stdfd(void); void
> > +update_display(struct modeset_dev *dev);
>
> Only the minority of symbols in the new library are namespaced, compare
> "platsch_format" against "draw" or "finish". Particularly the latter can be
> expected to be exported from third party code which libplatsch is then not able
> to be linked against. Thus please add the prefix "platsch_" to at least all symbols
> that are published via libplatsch.h.
>
> > +#endif
> > \ No newline at end of file
> > diff --git a/meson.build b/meson.build index 9f6be1e..dc55238 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -1,15 +1,18 @@
> >  project('platsch', 'c')
> >
> >  platsch_dep = dependency('libdrm', version : '>= 2.4.112', required :
> > true)
>
> One feature of platsch is to start up fast, hence it is intentionally linked against
> static libraries only. Therefore please use
>
>   platsch_dep = dependency('libdrm',
>       version: '>= 2.4.112',
>       static: true,
>       required: true
>   )
>
> instead.
>
> > -sources = ['platsch.c']
> > +sources = ['libplatsch.c']
> >
> > -# Define the headers
> > -headers = ['platsch.h']
> >
> > -# Create the platsch executable
> > -executable('platsch',
> > +libplatsch = static_library('libplatsch',
>
> The resulting library is called "liblibplatsch.a" now which is somewhat
> redundant.
>
> >      sources,
> >      dependencies: platsch_dep,
> > +)
> > +
> > +executable('platsch',
> > +    'platsch.c',
> > +    dependencies: platsch_dep,
> > +    link_with: libplatsch,
>
> Unfortunately picking the static variant of libdrm in its dependency()- statement
> is not enough to get a statically linked executable "platsch"
> as there are still some dynamically linked objects left, e.g. the c-library. Thus
> please add
>
>     link_args: '-static',

The current libdrm recipe of Yocto just generate dynamic link library,
Need to adaptor recipe to generate Static link library, this is the only one for current platsch.
But If involve cairo and png later, there would be more recipe to adaptor, like:
libgobject-2.0
libffi
libglib
libpcre
libpixman-1
libfontconfig
libuuid
libexpat
libfreetype
libpng16

do we really need to link all with static?

>
> to the executable() statement.
>
> Best regards
> Ulrich
>
>
> >      install: true,
> >      include_directories: include_directories('.')
> >  )
>
> [...]
>
> > -execinit:
> >       if (pid1) {
> >               ret = fork();
> >               if (ret < 0) {
> --
> Pengutronix e.K.                           | Ulrich Ölmann
> |
> Industrial Linux Solutions                 |
> http://www.pen/
> gutronix.de%2F&data=05%7C02%7Cqing-wu.li%40leica-geosystems.com.cn%7C
> 8fc4e5421e3d42d7ffef08dc8c60bec5%7C1b16ab3eb8f64fe39f3e2db7fe549f6a
> %7C0%7C0%7C638539594360937338%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C
> %7C%7C&sdata=NT9N%2FPYt2xioiQf4JRpwOgSqjrDHVKQ3tXhiDGEbDFk%3D&r
> eserved=0  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:
> +49-5121-206917-5555 |

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

end of thread, other threads:[~2024-06-19 12:57 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-13  7:07 [OSS-Tools] [PATCH platsch V3 1/4] platsch: constify draw_buffer LI Qingwu
2024-06-13  7:07 ` [OSS-Tools] [PATCH platsch V3 2/4] convert to meson build LI Qingwu
2024-06-13  8:44   ` Michael Tretter
2024-06-14  5:52   ` Ulrich Ölmann
2024-06-13  7:07 ` [OSS-Tools] [PATCH platsch V3 3/4] platsch: split into platsch and libplatsch LI Qingwu
2024-06-14 10:57   ` Ulrich Ölmann
2024-06-17  6:44     ` LI Qingwu
2024-06-13  7:07 ` [OSS-Tools] [PATCH platsch V3 4/4] Add spinner executable for boot animation and text show LI Qingwu
2024-06-14  5:30 ` [OSS-Tools] [PATCH platsch V3 1/4] platsch: constify draw_buffer Ulrich Ölmann

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