All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2 1/5] drivers: Add OSD uclass
@ 2018-05-23 12:09 Mario Six
  2018-05-23 12:09 ` [U-Boot] [PATCH v2 2/5] video_osd: Add ihs_video_out driver Mario Six
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Mario Six @ 2018-05-23 12:09 UTC (permalink / raw)
  To: u-boot

Some devices offer a text-based OSD (on-screen display) that can be
programmatically controlled (i.e. text displayed on).

Add a uclass to support such devices.

Signed-off-by: Mario Six <mario.six@gdsys.cc>

---

v1 -> v2:
* Use singular case for UCLASS_VIDEO_OSD description
* Expanded description of video_osd_set_mem with example
* Replaced all left-over mentions of pixels with text
* Renamed x and y parameters to col and row
* Expaneded description of color parameter
* Added general description of the OSD uclass
* Expanded description of video_osd_info

---
 drivers/video/Kconfig            |   8 ++
 drivers/video/Makefile           |   2 +
 drivers/video/video_osd-uclass.c |  46 ++++++++++
 include/dm/uclass-id.h           |   1 +
 include/video_osd.h              | 193 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 250 insertions(+)
 create mode 100644 drivers/video/video_osd-uclass.c
 create mode 100644 include/video_osd.h

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 4c4d2861fe7..37be23d2fcc 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -658,4 +658,12 @@ config VIDEO_DT_SIMPLEFB
 	  The video output is initialized by U-Boot, and kept by the
 	  kernel.

+config OSD
+	bool "Enable OSD support"
+	depends on DM
+	default n
+	help
+	   This supports drivers that provide a OSD (on-screen display), which
+	   is a (usually text-oriented) graphics buffer to show information on
+	   a display.
 endmenu
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index cf7ad281c3b..1889c5a5208 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -55,5 +55,7 @@ obj-${CONFIG_EXYNOS_FB} += exynos/
 obj-${CONFIG_VIDEO_ROCKCHIP} += rockchip/
 obj-${CONFIG_VIDEO_STM32} += stm32/

+obj-${CONFIG_OSD} += video_osd-uclass.o
+
 obj-y += bridge/
 obj-y += sunxi/
diff --git a/drivers/video/video_osd-uclass.c b/drivers/video/video_osd-uclass.c
new file mode 100644
index 00000000000..ae9b6a6fa82
--- /dev/null
+++ b/drivers/video/video_osd-uclass.c
@@ -0,0 +1,46 @@
+/*
+ * (C) Copyright 2017
+ * Mario Six,  Guntermann & Drunck GmbH, mario.six at gdsys.cc
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <video_osd.h>
+
+int video_osd_get_info(struct udevice *dev, struct video_osd_info *info)
+{
+	struct video_osd_ops *ops = video_osd_get_ops(dev);
+
+	return ops->get_info(dev, info);
+}
+
+int video_osd_set_mem(struct udevice *dev, uint col, uint row, u8 *buf,
+		      size_t buflen, uint count)
+{
+	struct video_osd_ops *ops = video_osd_get_ops(dev);
+
+	return ops->set_mem(dev, col, row, buf, buflen, count);
+}
+
+int video_osd_set_size(struct udevice *dev, uint col, uint row)
+{
+	struct video_osd_ops *ops = video_osd_get_ops(dev);
+
+	return ops->set_size(dev, col, row);
+}
+
+int video_osd_print(struct udevice *dev, uint col, uint row, ulong color,
+		    char *text)
+{
+	struct video_osd_ops *ops = video_osd_get_ops(dev);
+
+	return ops->print(dev, col, row, color, text);
+}
+
+UCLASS_DRIVER(video_osd) = {
+	.id		= UCLASS_VIDEO_OSD,
+	.name		= "video_osd",
+	.flags		= DM_UC_FLAG_SEQ_ALIAS,
+};
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index d7f9df3583a..e42f8481bf8 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -89,6 +89,7 @@ enum uclass_id {
 	UCLASS_VIDEO,		/* Video or LCD device */
 	UCLASS_VIDEO_BRIDGE,	/* Video bridge, e.g. DisplayPort to LVDS */
 	UCLASS_VIDEO_CONSOLE,	/* Text console driver for video device */
+	UCLASS_VIDEO_OSD,	/* On-screen display */
 	UCLASS_WDT,		/* Watchdot Timer driver */

 	UCLASS_COUNT,
diff --git a/include/video_osd.h b/include/video_osd.h
new file mode 100644
index 00000000000..c3bcd3a4fd8
--- /dev/null
+++ b/include/video_osd.h
@@ -0,0 +1,193 @@
+/*
+ * (C) Copyright 2017
+ * Mario Six,  Guntermann & Drunck GmbH, mario.six at gdsys.cc
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _VIDEO_OSD_H_
+#define _VIDEO_OSD_H_
+
+struct video_osd_info {
+	/* The width of the OSD display in columns */
+	uint width;
+	/* The height of the OSD display in rows */
+	uint height;
+	/* The major version of the OSD device */
+	uint major_version;
+	/* The minor version of the OSD device */
+	uint minor_version;
+};
+
+/**
+ * struct video_osd_ops - driver operations for OSD uclass
+ *
+ * The OSD uclass implements support for text-oriented on-screen displays,
+ * which are taken to be devices that independently display a graphical
+ * text-based overlay over the video output of an associated display.
+ *
+ * The functions defined by the uclass support writing text to the display in
+ * either a generic form (by specifying a string, a driver-specific color value
+ * for the text, and screen coordinates in rows and columns) or a
+ * driver-specific form (by specifying "raw" driver-specific data to display at
+ * a given coordinate).
+ *
+ * Functions to read device information and set the size of the virtual OSD
+ * screen (in rows and columns) are also supported.
+ *
+ * Drivers should support these operations unless otherwise noted. These
+ * operations are intended to be used by uclass code, not directly from
+ * other code.
+ */
+struct video_osd_ops {
+	/**
+	 * get_info() - Get information about a OSD instance
+	 *
+	 * A OSD instance may keep some internal data about itself. This
+	 * function can be used to access this data.
+	 *
+	 * @dev:	OSD instance to query.
+	 * @info:	Pointer to a structure that takes the information read
+	 *		from the OSD instance.
+	 * @return 0 if OK, -ve on error.
+	 */
+	int (*get_info)(struct udevice *dev, struct video_osd_info *info);
+
+	/**
+	 * set_mem() - Write driver-specific text data to OSD screen
+	 *
+	 * The passed data are device-specific, and it's up to the driver how
+	 * to interpret them. How the count parameter is interpreted is also
+	 * driver-specific; most likely the given data will be written to the
+	 * OSD count times back-to-back, which is e.g. convenient for filling
+	 * areas of the OSD with a single character.
+	 *
+	 * For example a invocation of
+	 *
+	 * video_osd_set_mem(dev, 0, 0, "A", 1, 10);
+	 *
+	 * will write the device-specific text data "A" to the positions (0, 0)
+	 * to (9, 0) on the OSD.
+	 *
+	 * Device-specific text data may, e.g. be a special encoding of glyphs
+	 * to display and color values in binary format.
+	 *
+	 * @dev:	OSD instance to write to.
+	 * @col:	Horizontal character coordinate to write to.
+	 * @row		Vertical character coordinate to write to.
+	 * @buf:	Array containing device-specific data to write to the
+	 *		specified coordinate on the OSD screen.
+	 * @buflen:	Length of the data in the passed buffer (in byte).
+	 * @count:	Write count many repetitions of the given text data
+	 * @return 0 if OK, -ve on error.
+	 */
+	int (*set_mem)(struct udevice *dev, uint col, uint row, u8 *buf,
+		       size_t buflen, uint count);
+
+	/**
+	 * set_size() - Set the position and dimension of the OSD's
+	 *              writeable window
+	 *
+	 * @dev:	OSD instance to write to.
+	 * @col		The number of characters in the window's columns
+	 * @row		The number of characters in the window's rows
+	 * @return 0 if OK, -ve on error.
+	 */
+	int (*set_size)(struct udevice *dev, uint col, uint row);
+
+	/**
+	 * print() - Print a string in a given color to specified coordinates
+	 *	     on the OSD
+	 *
+	 * @dev:	OSD instance to write to.
+	 * @col		The x-coordinate of the position the string should be
+	 *		written to
+	 * @row		The y-coordinate of the position the string should be
+	 *		written to
+	 * @color:	The color in which the specified string should be
+	 *		printed; the interpretation of the value is
+	 *		driver-specific, and possible values should be defined
+	 *		e.g. in a driver include file.
+	 * @text:	The string data that should be printed on the OSD
+	 * @return 0 if OK, -ve on error.
+	 */
+	int (*print)(struct udevice *dev, uint col, uint row, ulong color,
+		     char *text);
+};
+
+#define video_osd_get_ops(dev)	((struct video_osd_ops *)(dev)->driver->ops)
+
+/**
+ * video_osd_get_info() - Get information about a OSD instance
+ *
+ * A OSD instance may keep some internal data about itself. This function can
+ * be used to access this data.
+ *
+ * @dev:	OSD instance to query.
+ * @info:	Pointer to a structure that takes the information read from the
+ *		OSD instance.
+ * @return 0 if OK, -ve on error.
+ */
+int video_osd_get_info(struct udevice *dev, struct video_osd_info *info);
+
+/**
+ * video_osd_set_mem() - Write text data to OSD memory
+ *
+ * The passed data are device-specific, and it's up to the driver how to
+ * interpret them. How the count parameter is interpreted is also
+ * driver-specific; most likely the given data will be written to the OSD count
+ * times back-to-back, which is e.g. convenient for filling areas of the OSD
+ * with a single character.
+ *
+ * For example a invocation of
+ *
+ * video_osd_set_mem(dev, 0, 0, "A", 1, 10);
+ *
+ * will write the device-specific text data "A" to the positions (0, 0) to (9,
+ * 0) on the OSD.
+ *
+ * Device-specific text data may, e.g. be a special encoding of glyphs to
+ * display and color values in binary format.
+ *
+ * @dev:	OSD instance to write to.
+ * @col:	Horizontal character coordinate to write to.
+ * @row		Vertical character coordinate to write to.
+ * @buf:	Array containing device-specific data to write to the specified
+ *		coordinate on the OSD screen.
+ * @buflen:	Length of the data in the passed buffer (in byte).
+ * @count:	Write count many repetitions of the given text data
+ * @return 0 if OK, -ve on error.
+ */
+int video_osd_set_mem(struct udevice *dev, uint col, uint row, u8 *buf,
+		      size_t buflen, uint count);
+
+/**
+ * video_osd_set_size() - Set the position and dimension of the OSD's
+ *              writeable window
+ *
+ * @dev:	OSD instance to write to.
+ * @col		The number of characters in the window's columns
+ * @row		The number of characters in the window's rows
+ * @return 0 if OK, -ve on error.
+ */
+int video_osd_set_size(struct udevice *dev, uint col, uint row);
+
+/**
+ * video_osd_print() - Print a string in a given color to specified coordinates
+ *		       on the OSD
+ *
+ * @dev:	OSD instance to write to.
+ * @col		The x-coordinate of the position the string should be written
+ *		to
+ * @row		The y-coordinate of the position the string should be written
+ *		to
+ * @color:	The color in which the specified string should be printed; the
+ *		interpretation of the value is driver-specific, and possible
+ *		values should be defined e.g. in a driver include file.
+ * @text:	The string data that should be printed on the OSD
+ * @return 0 if OK, -ve on error.
+ */
+int video_osd_print(struct udevice *dev, uint col, uint row, ulong color,
+		    char *text);
+
+#endif /* !_VIDEO_OSD_H_ */
--
2.11.0

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

* [U-Boot] [PATCH v2 2/5] video_osd: Add ihs_video_out driver
  2018-05-23 12:09 [U-Boot] [PATCH v2 1/5] drivers: Add OSD uclass Mario Six
@ 2018-05-23 12:09 ` Mario Six
  2018-05-23 16:33   ` Simon Glass
  2018-05-23 12:09 ` [U-Boot] [PATCH v2 3/5] video_osd: Add osd sandbox driver and tests Mario Six
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Mario Six @ 2018-05-23 12:09 UTC (permalink / raw)
  To: u-boot

Add a driver for IHS OSDs on IHS FPGAs.

Signed-off-by: Mario Six <mario.six@gdsys.cc>

---

v1 -> v2:
* Renamed x and y parameters to col and row
* Removed duplicate IHS_VIDEO_OUT definition
* Switched error return code of ihs_video_out_set_mem to E2BIG
* Dropped blank lines between dev_read_u32_default calls
* Documented members of ihs_video_out_priv
* Turned printfs into debugs
* Don't display OSD until something has been written to it
* Made the code a bit more readable
* Added binding file
* Expanded documentation and help
* Switched to display_enable
* Switched to regmap usage (instead of fpgamap uclass)
* Renamed 'dp_tx' property to 'video_tx'

---
 .../video/osd/gdsys,ihs_video_out.txt              |  22 ++
 drivers/video/Kconfig                              |   9 +
 drivers/video/Makefile                             |   1 +
 drivers/video/ihs_video_out.c                      | 277 +++++++++++++++++++++
 4 files changed, 309 insertions(+)
 create mode 100644 doc/device-tree-bindings/video/osd/gdsys,ihs_video_out.txt
 create mode 100644 drivers/video/ihs_video_out.c

diff --git a/doc/device-tree-bindings/video/osd/gdsys,ihs_video_out.txt b/doc/device-tree-bindings/video/osd/gdsys,ihs_video_out.txt
new file mode 100644
index 00000000000..464374613b8
--- /dev/null
+++ b/doc/device-tree-bindings/video/osd/gdsys,ihs_video_out.txt
@@ -0,0 +1,22 @@
+* Guntermann & Drunck Integrated Hardware Systems OSD
+
+Required properties:
+- compatible: "gdsys,ihs_video_out"
+- reg: Register base for the video registers
+- osd_base: Register base for the OSD registers
+- osd_buffer_base: Address of the OSD video memory
+- mode: The initial resolution and frequency: "1024_768_60", "720_400_70", or
+        "640_480_70"
+- clk_gen: phandle to the pixel clock generator
+- dp_tx: phandle to the display associated with the OSD
+
+Example:
+
+fpga0_video0 {
+	compatible = "gdsys,ihs_video_out";
+	reg = <0x100 0x40>;
+	osd_base = <0x180>;
+	osd_buffer_base = <0x1000>;
+	dp_tx = <&fpga0_dp_video0>;
+	clk_gen = <&fpga0_video0_clkgen>;
+};
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 37be23d2fcc..4ca15174bb2 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -666,4 +666,13 @@ config OSD
 	   This supports drivers that provide a OSD (on-screen display), which
 	   is a (usually text-oriented) graphics buffer to show information on
 	   a display.
+
+config IHS_VIDEO_OUT
+	bool "Enable IHS video out driver"
+	depends on OSD
+	help
+	  Enable support for the gdsys Integrated Hardware Systems (IHS) video
+	  out On-screen Display (OSD) used on gdsys FPGAs to control dynamic
+	  textual overlays of the display outputs.
+
 endmenu
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 1889c5a5208..44252b5989a 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -56,6 +56,7 @@ obj-${CONFIG_VIDEO_ROCKCHIP} += rockchip/
 obj-${CONFIG_VIDEO_STM32} += stm32/

 obj-${CONFIG_OSD} += video_osd-uclass.o
+obj-$(CONFIG_IHS_VIDEO_OUT) += ihs_video_out.o

 obj-y += bridge/
 obj-y += sunxi/
diff --git a/drivers/video/ihs_video_out.c b/drivers/video/ihs_video_out.c
new file mode 100644
index 00000000000..3a870e22c2e
--- /dev/null
+++ b/drivers/video/ihs_video_out.c
@@ -0,0 +1,277 @@
+/*
+ * (C) Copyright 2017
+ * Mario Six, Guntermann & Drunck GmbH, mario.six at gdsys.cc
+ *
+ * based on the gdsys osd driver, which is
+ *
+ * (C) Copyright 2010
+ * Dirk Eibach, Guntermann & Drunck GmbH, dirk.eibach at gdsys.de
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <display.h>
+#include <dm.h>
+#include <regmap.h>
+#include <video_osd.h>
+#include <asm/gpio.h>
+
+#define MAX_X_CHARS 53
+#define MAX_Y_CHARS 26
+#define MAX_VIDEOMEM_WIDTH (2 * 64)
+#define MAX_VIDEOMEM_HEIGHT (2 * 32)
+#define CHAR_WIDTH 12
+#define CHAR_HEIGHT 18
+
+struct ihs_video_out_regs {
+	u16 versions;
+	u16 features;
+	u16 control;
+	u16 xy_size;
+	u16 xy_scale;
+	u16 x_pos;
+	u16 y_pos;
+};
+
+#define ihs_video_out_set(map, member, val) \
+	regmap_range_set(map, 1, struct ihs_video_out_regs, member, val)
+
+#define ihs_video_out_get(map, member, valp) \
+	regmap_range_get(map, 1, struct ihs_video_out_regs, member, valp)
+
+enum {
+	CONTROL_FILTER_BLACK = (0 << 0),
+	CONTROL_FILTER_ORIGINAL = (1 << 0),
+	CONTROL_FILTER_DARKER = (2 << 0),
+	CONTROL_FILTER_GRAY = (3 << 0),
+
+	CONTROL_MODE_PASSTHROUGH = (0 << 3),
+	CONTROL_MODE_OSD = (1 << 3),
+	CONTROL_MODE_AUTO = (2 << 3),
+	CONTROL_MODE_OFF = (3 << 3),
+
+	CONTROL_ENABLE_OFF = (0 << 6),
+	CONTROL_ENABLE_ON = (1 << 6),
+};
+
+struct ihs_video_out_priv {
+	/* Register map for OSD device */
+	struct regmap *map;
+	/* Pointer to video memory */
+	u16 *vidmem;
+	/* Display width in text columns */
+	uint base_width;
+	/* Display height in text rows */
+	uint base_height;
+	/* x-resolution of the display in pixels */
+	uint res_x;
+	/* y-resolution of the display in pixels */
+	uint res_y;
+	/* OSD's sync mode (resolution + frequency) */
+	int sync_src;
+	/* The display port output for this OSD */
+	struct udevice *video_tx;
+	/* The pixel clock generator for the display */
+	struct udevice *clk_gen;
+};
+
+static const struct udevice_id ihs_video_out_ids[] = {
+	{ .compatible = "gdsys,ihs_video_out" },
+	{ }
+};
+
+static int set_control(struct udevice *dev, u16 value)
+{
+	struct ihs_video_out_priv *priv = dev_get_priv(dev);
+
+	if (priv->sync_src)
+		value |= ((priv->sync_src & 0x7) << 8);
+
+	ihs_video_out_set(priv->map, control, value);
+
+	return 0;
+}
+
+int ihs_video_out_get_info(struct udevice *dev, struct video_osd_info *info)
+{
+	struct ihs_video_out_priv *priv = dev_get_priv(dev);
+	u16 versions;
+
+	ihs_video_out_get(priv->map, versions, &versions);
+
+	info->width = priv->base_width;
+	info->height = priv->base_height;
+	info->major_version = versions / 100;
+	info->minor_version = versions % 100;
+
+	return 0;
+}
+
+int ihs_video_out_set_mem(struct udevice *dev, uint col, uint row, u8 *buf,
+			  size_t buflen, uint count)
+{
+	struct ihs_video_out_priv *priv = dev_get_priv(dev);
+	uint offset;
+	uint k, l;
+	u16 data;
+
+	for (l = 0; l < count; ++l) {
+		offset = row * priv->base_width + col + l * (buflen / 2);
+
+		for (k = 0; k < buflen / 2; ++k) {
+			if (offset + k >= priv->base_width * priv->base_height)
+				return -E2BIG;
+
+			data = buf[2 * k + 1] + 256 * buf[2 * k];
+			out_le16(priv->vidmem + offset + k, data);
+		}
+	}
+
+	set_control(dev, CONTROL_FILTER_ORIGINAL |
+			 CONTROL_MODE_OSD |
+			 CONTROL_ENABLE_ON);
+
+	return 0;
+}
+
+inline u16 div2_u16(u16 val)
+{
+	return (32767 * val) / 65535;
+}
+
+int ihs_video_out_set_size(struct udevice *dev, uint col, uint row)
+{
+	struct ihs_video_out_priv *priv = dev_get_priv(dev);
+
+	if (!col || col > 64 || col > MAX_X_CHARS ||
+	    !row || row > 32 || row > MAX_Y_CHARS) {
+		return -EINVAL;
+	}
+
+	ihs_video_out_set(priv->map, xy_size, ((col - 1) << 8) | (row - 1));
+	/* Center OSD on screen */
+	ihs_video_out_set(priv->map, x_pos,
+			  div2_u16(priv->res_x - CHAR_WIDTH * col));
+	ihs_video_out_set(priv->map, y_pos,
+			  div2_u16(priv->res_y - CHAR_HEIGHT * row));
+
+	return 0;
+}
+
+int ihs_video_out_print(struct udevice *dev, uint col, uint row, ulong color,
+			char *text)
+{
+	int res;
+	u8 buffer[MAX_VIDEOMEM_WIDTH];
+	uint k;
+	uint charcount = strlen(text);
+	uint len = min(charcount, (uint)(MAX_VIDEOMEM_WIDTH));
+
+	for (k = 0; k < len; ++k) {
+		buffer[2 * k] = text[k];
+		buffer[2 * k + 1] = color;
+	}
+
+	res = ihs_video_out_set_mem(dev, col, row, buffer, 2 * len, 1);
+
+	if (res < 0)
+		return res;
+
+	return 0;
+}
+
+static const struct video_osd_ops ihs_video_out_ops = {
+	.get_info = ihs_video_out_get_info,
+	.set_mem = ihs_video_out_set_mem,
+	.set_size = ihs_video_out_set_size,
+	.print = ihs_video_out_print,
+};
+
+int ihs_video_out_probe(struct udevice *dev)
+{
+	struct ihs_video_out_priv *priv = dev_get_priv(dev);
+	int len = 0;
+	struct ofnode_phandle_args phandle_args;
+	char *mode;
+	u16 features;
+	struct display_timing timing;
+
+	regmap_init_mem(dev_ofnode(dev), &priv->map);
+
+	/* Range with index 2 is video memory */
+	priv->vidmem = regmap_get_range(priv->map, 2);
+
+	mode = (char *)dev_read_prop(dev, "mode", &len);
+
+	if (!mode) {
+		debug("%s: Could not read mode property.\n", dev->name);
+		return -EINVAL;
+	}
+
+	if (!strcmp(mode, "1024_768_60")) {
+		priv->sync_src = 2;
+		priv->res_x = 1024;
+		priv->res_y = 768;
+		timing.hactive.typ = 1024;
+		timing.vactive.typ = 768;
+	} else if (!strcmp(mode, "720_400_70")) {
+		priv->sync_src = 1;
+		priv->res_x = 720;
+		priv->res_y = 400;
+		timing.hactive.typ = 720;
+		timing.vactive.typ = 400;
+	} else {
+		priv->sync_src = 0;
+		priv->res_x = 640;
+		priv->res_y = 480;
+		timing.hactive.typ = 640;
+		timing.vactive.typ = 480;
+	}
+
+	ihs_video_out_get(priv->map, features, &features);
+
+	set_control(dev, CONTROL_FILTER_ORIGINAL |
+			 CONTROL_MODE_OSD |
+			 CONTROL_ENABLE_OFF);
+
+	priv->base_width = ((features & 0x3f00) >> 8) + 1;
+	priv->base_height = (features & 0x001f) + 1;
+
+	if (dev_read_phandle_with_args(dev, "clk_gen", NULL, 0, 0,
+				       &phandle_args)) {
+		debug("%s: Could not get clk_gen node.\n", dev->name);
+		return -EINVAL;
+	}
+
+	if (uclass_get_device_by_ofnode(UCLASS_CLK, phandle_args.node,
+					&priv->clk_gen)) {
+		debug("%s: Could not get clk_gen dev.\n", dev->name);
+		return -EINVAL;
+	}
+
+	if (dev_read_phandle_with_args(dev, "video_tx", NULL, 0, 0,
+				       &phandle_args)) {
+		debug("%s: Could not get video_tx.\n", dev->name);
+		return -EINVAL;
+	}
+
+	if (uclass_get_device_by_ofnode(UCLASS_DISPLAY, phandle_args.node,
+					&priv->video_tx)) {
+		debug("%s: Could not get video_tx dev.\n", dev->name);
+		return -EINVAL;
+	}
+
+	display_enable(priv->video_tx, 8, &timing);
+
+	return 0;
+}
+
+U_BOOT_DRIVER(ihs_video_out_drv) = {
+	.name           = "ihs_video_out_drv",
+	.id             = UCLASS_VIDEO_OSD,
+	.ops		= &ihs_video_out_ops,
+	.of_match       = ihs_video_out_ids,
+	.probe          = ihs_video_out_probe,
+	.priv_auto_alloc_size = sizeof(struct ihs_video_out_priv),
+};
--
2.11.0

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

* [U-Boot] [PATCH v2 3/5] video_osd: Add osd sandbox driver and tests
  2018-05-23 12:09 [U-Boot] [PATCH v2 1/5] drivers: Add OSD uclass Mario Six
  2018-05-23 12:09 ` [U-Boot] [PATCH v2 2/5] video_osd: Add ihs_video_out driver Mario Six
@ 2018-05-23 12:09 ` Mario Six
  2018-05-23 16:33   ` Simon Glass
  2018-05-23 12:09 ` [U-Boot] [PATCH v2 4/5] lib: Import hexdump.c from Linux Mario Six
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Mario Six @ 2018-05-23 12:09 UTC (permalink / raw)
  To: u-boot

Add sandbox driver and tests for the new OSD uclass.

Signed-off-by: Mario Six <mario.six@gdsys.cc>

---

v1 -> v2:
New in v2

---
 arch/sandbox/dts/test.dts          |   4 +
 configs/sandbox64_defconfig        |   3 +
 configs/sandbox_defconfig          |   3 +
 configs/sandbox_flattree_defconfig |   3 +
 configs/sandbox_noblk_defconfig    |   3 +
 configs/sandbox_spl_defconfig      |   3 +
 drivers/video/Kconfig              |   6 ++
 drivers/video/Makefile             |   1 +
 drivers/video/sandbox_osd.c        | 157 ++++++++++++++++++++++++++++
 drivers/video/sandbox_osd.h        |  14 +++
 drivers/video/video_osd-uclass.c   |   9 ++
 include/video_osd.h                |   7 ++
 test/dm/Makefile                   |   1 +
 test/dm/osd.c                      | 208 +++++++++++++++++++++++++++++++++++++
 14 files changed, 422 insertions(+)
 create mode 100644 drivers/video/sandbox_osd.c
 create mode 100644 drivers/video/sandbox_osd.h
 create mode 100644 test/dm/osd.c

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 5a0f187d8b7..b6cdda0fd33 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -559,6 +559,10 @@
 			};
 		};
 	};
+
+	osd {
+		compatible = "sandbox,sandbox_osd";
+	};
 };

 #include "sandbox_pmic.dtsi"
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 20a2ab3ffb7..7464a3ca3af 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -39,6 +39,7 @@ CONFIG_CMD_GPT=y
 CONFIG_CMD_GPT_RENAME=y
 CONFIG_CMD_IDE=y
 CONFIG_CMD_I2C=y
+CONFIG_CMD_OSD=y
 CONFIG_CMD_PCI=y
 CONFIG_CMD_READ=y
 CONFIG_CMD_REMOTEPROC=y
@@ -185,6 +186,8 @@ CONFIG_CONSOLE_ROTATION=y
 CONFIG_CONSOLE_TRUETYPE=y
 CONFIG_CONSOLE_TRUETYPE_CANTORAONE=y
 CONFIG_VIDEO_SANDBOX_SDL=y
+CONFIG_OSD=y
+CONFIG_SANDBOX_OSD=y
 CONFIG_WDT=y
 CONFIG_WDT_SANDBOX=y
 CONFIG_FS_CBFS=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 2fc84a16c91..f7925dfe6fb 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -39,6 +39,7 @@ CONFIG_CMD_GPT=y
 CONFIG_CMD_GPT_RENAME=y
 CONFIG_CMD_IDE=y
 CONFIG_CMD_I2C=y
+CONFIG_CMD_OSD=y
 CONFIG_CMD_PCI=y
 CONFIG_CMD_READ=y
 CONFIG_CMD_REMOTEPROC=y
@@ -186,6 +187,8 @@ CONFIG_CONSOLE_ROTATION=y
 CONFIG_CONSOLE_TRUETYPE=y
 CONFIG_CONSOLE_TRUETYPE_CANTORAONE=y
 CONFIG_VIDEO_SANDBOX_SDL=y
+CONFIG_OSD=y
+CONFIG_SANDBOX_OSD=y
 CONFIG_WDT=y
 CONFIG_WDT_SANDBOX=y
 CONFIG_FS_CBFS=y
diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig
index e922c4b38ff..126b9c07c54 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -31,6 +31,7 @@ CONFIG_CMD_DEMO=y
 CONFIG_CMD_GPIO=y
 CONFIG_CMD_GPT=y
 CONFIG_CMD_I2C=y
+CONFIG_CMD_OSD=y
 CONFIG_CMD_PCI=y
 CONFIG_CMD_REMOTEPROC=y
 CONFIG_CMD_SF=y
@@ -167,6 +168,8 @@ CONFIG_CONSOLE_ROTATION=y
 CONFIG_CONSOLE_TRUETYPE=y
 CONFIG_CONSOLE_TRUETYPE_CANTORAONE=y
 CONFIG_VIDEO_SANDBOX_SDL=y
+CONFIG_OSD=y
+CONFIG_SANDBOX_OSD=y
 CONFIG_CMD_DHRYSTONE=y
 CONFIG_TPM=y
 CONFIG_LZ4=y
diff --git a/configs/sandbox_noblk_defconfig b/configs/sandbox_noblk_defconfig
index 8bdd4edcda6..243234b6c72 100644
--- a/configs/sandbox_noblk_defconfig
+++ b/configs/sandbox_noblk_defconfig
@@ -35,6 +35,7 @@ CONFIG_CMD_GPIO=y
 CONFIG_CMD_GPT=y
 CONFIG_CMD_IDE=y
 CONFIG_CMD_I2C=y
+CONFIG_CMD_OSD=y
 CONFIG_CMD_PCI=y
 CONFIG_CMD_REMOTEPROC=y
 CONFIG_CMD_SF=y
@@ -166,6 +167,8 @@ CONFIG_CONSOLE_ROTATION=y
 CONFIG_CONSOLE_TRUETYPE=y
 CONFIG_CONSOLE_TRUETYPE_CANTORAONE=y
 CONFIG_VIDEO_SANDBOX_SDL=y
+CONFIG_OSD=y
+CONFIG_SANDBOX_OSD=y
 CONFIG_FS_CBFS=y
 CONFIG_FS_CRAMFS=y
 CONFIG_CMD_DHRYSTONE=y
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
index fb6bb4baa2b..771d0f05a48 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -43,6 +43,7 @@ CONFIG_CMD_GPIO=y
 CONFIG_CMD_GPT=y
 CONFIG_CMD_IDE=y
 CONFIG_CMD_I2C=y
+CONFIG_CMD_OSD=y
 CONFIG_CMD_PCI=y
 CONFIG_CMD_REMOTEPROC=y
 CONFIG_CMD_SF=y
@@ -185,6 +186,8 @@ CONFIG_CONSOLE_ROTATION=y
 CONFIG_CONSOLE_TRUETYPE=y
 CONFIG_CONSOLE_TRUETYPE_CANTORAONE=y
 CONFIG_VIDEO_SANDBOX_SDL=y
+CONFIG_OSD=y
+CONFIG_SANDBOX_OSD=y
 CONFIG_FS_CBFS=y
 CONFIG_FS_CRAMFS=y
 CONFIG_CMD_DHRYSTONE=y
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 4ca15174bb2..b1a8e05ad74 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -667,6 +667,12 @@ config OSD
 	   is a (usually text-oriented) graphics buffer to show information on
 	   a display.

+config SANDBOX_OSD
+	bool "Enable sandbox OSD"
+	depends on OSD
+	help
+	  Enable support for sandbox OSD device used for testing purposes.
+
 config IHS_VIDEO_OUT
 	bool "Enable IHS video out driver"
 	depends on OSD
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 44252b5989a..80a8b19ed5d 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -56,6 +56,7 @@ obj-${CONFIG_VIDEO_ROCKCHIP} += rockchip/
 obj-${CONFIG_VIDEO_STM32} += stm32/

 obj-${CONFIG_OSD} += video_osd-uclass.o
+obj-${CONFIG_SANDBOX_OSD} += sandbox_osd.o
 obj-$(CONFIG_IHS_VIDEO_OUT) += ihs_video_out.o

 obj-y += bridge/
diff --git a/drivers/video/sandbox_osd.c b/drivers/video/sandbox_osd.c
new file mode 100644
index 00000000000..4fdae1bbedd
--- /dev/null
+++ b/drivers/video/sandbox_osd.c
@@ -0,0 +1,157 @@
+#include <common.h>
+#include <display.h>
+#include <dm.h>
+#include <video_osd.h>
+
+#include "sandbox_osd.h"
+
+struct sandbox_osd_priv {
+	uint width;
+	uint height;
+	u16 *buf;
+};
+
+static const struct udevice_id sandbox_osd_ids[] = {
+	{ .compatible = "sandbox,sandbox_osd" },
+	{ }
+};
+
+inline u16 make_memval(u8 chr, u8 color)
+{
+	return chr * 0x100 + color;
+}
+
+int sandbox_osd_get_info(struct udevice *dev, struct video_osd_info *info)
+{
+	struct sandbox_osd_priv *priv = dev_get_priv(dev);
+
+	info->width = priv->width;
+	info->height = priv->height;
+	info->major_version = 1;
+	info->minor_version = 0;
+
+	return 0;
+}
+
+int sandbox_osd_set_mem(struct udevice *dev, uint col, uint row, u8 *buf,
+			size_t buflen, uint count)
+{
+	struct sandbox_osd_priv *priv = dev_get_priv(dev);
+	int pos;
+	u8 *mem = (u8 *)priv->buf;
+	int i;
+
+	pos = 2 * (row * priv->width + col);
+
+	if (pos >= 2 * (priv->width * priv->height))
+		return -EINVAL;
+
+	for (i = 0; i < count; i++)
+		memcpy(mem + pos + (i * buflen), buf, buflen);
+
+	return 0;
+}
+
+int _sandbox_osd_set_size(struct udevice *dev, uint col, uint row)
+{
+	struct sandbox_osd_priv *priv = dev_get_priv(dev);
+	int i;
+	uint size;
+
+	priv->width = col;
+	priv->height = row;
+	size = priv->width * priv->height;
+	if (!priv->buf)
+		priv->buf = calloc(size, sizeof(u16));
+	else
+		priv->buf = realloc(priv->buf, size * sizeof(u16));
+
+	if (!priv->buf)
+		return -ENOMEM;
+
+	/* Fill OSD with black spaces */
+	for (i = 0; i < size; i++)
+		priv->buf[i] = make_memval(' ', 'k');
+
+	return 0;
+}
+
+int sandbox_osd_set_size(struct udevice *dev, uint col, uint row)
+{
+	return _sandbox_osd_set_size(dev, col, row);
+}
+
+int sandbox_osd_print(struct udevice *dev, uint col, uint row, ulong color,
+		      char *text)
+{
+	struct sandbox_osd_priv *priv = dev_get_priv(dev);
+	char cval;
+	char *p;
+	int pos;
+
+	if (col >= priv->width || row >= priv->height)
+		return -EINVAL;
+
+	switch (color) {
+	case COLOR_BLACK:
+		cval = 'k';
+		break;
+	case COLOR_WHITE:
+		cval = 'w';
+		break;
+	case COLOR_RED:
+		cval = 'r';
+		break;
+	case COLOR_GREEN:
+		cval = 'g';
+		break;
+	case COLOR_BLUE:
+		cval = 'b';
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	p = text;
+	pos = row * priv->width + col;
+
+	while (*p)
+		priv->buf[pos++] = make_memval(*(p++), cval);
+
+	return 0;
+}
+
+int sandbox_osd_get_mem(struct udevice *dev, u8 *buf, size_t buflen)
+{
+	struct sandbox_osd_priv *priv = dev_get_priv(dev);
+	uint memsize = 2 * (priv->width * priv->height);
+
+	if (buflen < memsize)
+		return -EINVAL;
+
+	memcpy(buf, priv->buf, memsize);
+
+	return 0;
+}
+
+static const struct video_osd_ops sandbox_osd_ops = {
+	.get_info = sandbox_osd_get_info,
+	.set_mem = sandbox_osd_set_mem,
+	.set_size = sandbox_osd_set_size,
+	.print = sandbox_osd_print,
+	.get_mem = sandbox_osd_get_mem,
+};
+
+int sandbox_osd_probe(struct udevice *dev)
+{
+	return _sandbox_osd_set_size(dev, 10, 10);
+}
+
+U_BOOT_DRIVER(sandbox_osd_drv) = {
+	.name           = "sandbox_osd_drv",
+	.id             = UCLASS_VIDEO_OSD,
+	.ops		= &sandbox_osd_ops,
+	.of_match       = sandbox_osd_ids,
+	.probe          = sandbox_osd_probe,
+	.priv_auto_alloc_size = sizeof(struct sandbox_osd_priv),
+};
diff --git a/drivers/video/sandbox_osd.h b/drivers/video/sandbox_osd.h
new file mode 100644
index 00000000000..644779913ec
--- /dev/null
+++ b/drivers/video/sandbox_osd.h
@@ -0,0 +1,14 @@
+/*
+ * (C) Copyright 2018
+ * Mario Six, Guntermann & Drunck GmbH, mario.six at gdsys.cc
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+enum {
+	COLOR_BLACK,
+	COLOR_WHITE,
+	COLOR_RED,
+	COLOR_GREEN,
+	COLOR_BLUE,
+};
diff --git a/drivers/video/video_osd-uclass.c b/drivers/video/video_osd-uclass.c
index ae9b6a6fa82..05886f3bd5b 100644
--- a/drivers/video/video_osd-uclass.c
+++ b/drivers/video/video_osd-uclass.c
@@ -39,6 +39,15 @@ int video_osd_print(struct udevice *dev, uint col, uint row, ulong color,
 	return ops->print(dev, col, row, color, text);
 }

+#ifdef CONFIG_SANDBOX
+int video_osd_get_mem(struct udevice *dev, u8 *buf, size_t buflen)
+{
+	struct video_osd_ops *ops = video_osd_get_ops(dev);
+
+	return ops->get_mem(dev, buf, buflen);
+}
+#endif
+
 UCLASS_DRIVER(video_osd) = {
 	.id		= UCLASS_VIDEO_OSD,
 	.name		= "video_osd",
diff --git a/include/video_osd.h b/include/video_osd.h
index c3bcd3a4fd8..006cf916c94 100644
--- a/include/video_osd.h
+++ b/include/video_osd.h
@@ -113,6 +113,9 @@ struct video_osd_ops {
 	 */
 	int (*print)(struct udevice *dev, uint col, uint row, ulong color,
 		     char *text);
+#ifdef CONFIG_SANDBOX
+	int (*get_mem)(struct udevice *dev, u8 *buf, size_t buflen);
+#endif
 };

 #define video_osd_get_ops(dev)	((struct video_osd_ops *)(dev)->driver->ops)
@@ -190,4 +193,8 @@ int video_osd_set_size(struct udevice *dev, uint col, uint row);
 int video_osd_print(struct udevice *dev, uint col, uint row, ulong color,
 		    char *text);

+#ifdef CONFIG_SANDBOX
+int video_osd_get_mem(struct udevice *dev, u8 *buf, size_t buflen);
+#endif
+
 #endif /* !_VIDEO_OSD_H_ */
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 5511a85700c..7b390e1d2a0 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_LED) += led.o
 obj-$(CONFIG_DM_MAILBOX) += mailbox.o
 obj-$(CONFIG_DM_MMC) += mmc.o
 obj-y += ofnode.o
+obj-$(CONFIG_OSD) += osd.o
 obj-$(CONFIG_DM_PCI) += pci.o
 obj-$(CONFIG_PHY) += phy.o
 obj-$(CONFIG_POWER_DOMAIN) += power-domain.o
diff --git a/test/dm/osd.c b/test/dm/osd.c
new file mode 100644
index 00000000000..807cb885077
--- /dev/null
+++ b/test/dm/osd.c
@@ -0,0 +1,208 @@
+/*
+ * (C) Copyright 2018
+ * Mario Six, Guntermann & Drunck GmbH, mario.six at gdsys.cc
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <dm/test.h>
+#include <video_osd.h>
+#include <display_options.h>
+#include <test/ut.h>
+
+#include "../../drivers/video/sandbox_osd.h"
+
+const uint memsize = 2 * 10 * 10;
+
+static void split(u8 *mem, uint size, u8 *text, u8 *colors)
+{
+	int i;
+	u16 *p = (u16 *)mem;
+
+	for (i = 0; i < size; i++) {
+		colors[i] = p[i] % 0x100;
+		text[i] = p[i] / 0x100;
+	}
+}
+
+static void print_mem(u8 *mem, uint width, uint height)
+{
+	const uint memsize = 2 * 10 * 10;
+	u8 colors[memsize / 2];
+	u8 text[memsize / 2];
+	int i;
+
+	split(mem, memsize / 2, text, colors);
+
+	for (i = 0; i < width * height; i++) {
+		printf("%c", text[i]);
+		if (i > 0 && ((i + 1) % width) == 0)
+			printf("\n");
+	}
+
+	printf("\n");
+
+	for (i = 0; i < width * height; i++) {
+		printf("%c", colors[i]);
+		if (i > 0 && ((i + 1) % width) == 0)
+			printf("\n");
+	}
+}
+
+static int dm_test_osd_basics(struct unit_test_state *uts)
+{
+	struct udevice *dev;
+	u8 mem[memsize + 1];
+	u8 colors[memsize / 2];
+	u8 text[memsize / 2];
+	struct video_osd_info info;
+
+	ut_assertok(uclass_first_device_err(UCLASS_VIDEO_OSD, &dev));
+
+	video_osd_get_info(dev, &info);
+
+	ut_asserteq(10, info.width);
+	ut_asserteq(10, info.height);
+	ut_asserteq(1, info.major_version);
+	ut_asserteq(0, info.minor_version);
+
+	ut_assertok(video_osd_get_mem(dev, mem, memsize));
+	split(mem, memsize / 2, text, colors);
+
+	ut_assertok(memcmp(text, "          "
+				 "          "
+				 "          "
+				 "          "
+				 "          "
+				 "          "
+				 "          "
+				 "          "
+				 "          "
+				 "          ", memsize / 2));
+
+	ut_assertok(memcmp(colors, "kkkkkkkkkk"
+				   "kkkkkkkkkk"
+				   "kkkkkkkkkk"
+				   "kkkkkkkkkk"
+				   "kkkkkkkkkk"
+				   "kkkkkkkkkk"
+				   "kkkkkkkkkk"
+				   "kkkkkkkkkk"
+				   "kkkkkkkkkk"
+				   "kkkkkkkkkk", memsize / 2));
+
+	print_mem(mem, 10, 10);
+
+	ut_assertok(video_osd_print(dev, 1, 1, COLOR_RED, "Blah"));
+
+	ut_assertok(video_osd_get_mem(dev, mem, memsize));
+	split(mem, memsize / 2, text, colors);
+
+	ut_assertok(memcmp(text, "          "
+				 " Blah     "
+				 "          "
+				 "          "
+				 "          "
+				 "          "
+				 "          "
+				 "          "
+				 "          "
+				 "          ", memsize / 2));
+
+	ut_assertok(memcmp(colors, "kkkkkkkkkk"
+				   "krrrrkkkkk"
+				   "kkkkkkkkkk"
+				   "kkkkkkkkkk"
+				   "kkkkkkkkkk"
+				   "kkkkkkkkkk"
+				   "kkkkkkkkkk"
+				   "kkkkkkkkkk"
+				   "kkkkkkkkkk"
+				   "kkkkkkkkkk", memsize / 2));
+
+	print_mem(mem, 10, 10);
+
+	return 0;
+}
+DM_TEST(dm_test_osd_basics, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
+static int dm_test_osd_extended(struct unit_test_state *uts)
+{
+	struct udevice *dev;
+	u8 mem[memsize + 1];
+	u8 colors[memsize / 2];
+	u8 text[memsize / 2];
+	struct video_osd_info info;
+	u16 val;
+
+	ut_assertok(uclass_first_device_err(UCLASS_VIDEO_OSD, &dev));
+
+	ut_assertok(video_osd_set_size(dev, 20, 5));
+
+	video_osd_get_info(dev, &info);
+
+	ut_asserteq(20, info.width);
+	ut_asserteq(5, info.height);
+	ut_asserteq(1, info.major_version);
+	ut_asserteq(0, info.minor_version);
+
+	ut_assertok(video_osd_get_mem(dev, mem, memsize));
+	split(mem, memsize / 2, text, colors);
+
+	ut_assertok(memcmp(text, "                    "
+				 "                    "
+				 "                    "
+				 "                    "
+				 "                    ", memsize / 2));
+
+	ut_assertok(memcmp(colors, "kkkkkkkkkkkkkkkkkkkk"
+				   "kkkkkkkkkkkkkkkkkkkk"
+				   "kkkkkkkkkkkkkkkkkkkk"
+				   "kkkkkkkkkkkkkkkkkkkk"
+				   "kkkkkkkkkkkkkkkkkkkk", memsize / 2));
+
+	print_mem(mem, 20, 5);
+
+	/* Draw green border */
+	val = '-' * 0x100 + 'g';
+	ut_assertok(video_osd_set_mem(dev, 1, 0, (u8 *)&val, 2, 18));
+	ut_assertok(video_osd_set_mem(dev, 1, 4, (u8 *)&val, 2, 18));
+	ut_assertok(video_osd_print(dev, 0, 1, COLOR_GREEN, "|"));
+	ut_assertok(video_osd_print(dev, 0, 2, COLOR_GREEN, "|"));
+	ut_assertok(video_osd_print(dev, 0, 3, COLOR_GREEN, "|"));
+	ut_assertok(video_osd_print(dev, 19, 1, COLOR_GREEN, "|"));
+	ut_assertok(video_osd_print(dev, 19, 2, COLOR_GREEN, "|"));
+	ut_assertok(video_osd_print(dev, 19, 3, COLOR_GREEN, "|"));
+	ut_assertok(video_osd_print(dev, 0, 0, COLOR_GREEN, "+"));
+	ut_assertok(video_osd_print(dev, 19, 0, COLOR_GREEN, "+"));
+	ut_assertok(video_osd_print(dev, 19, 4, COLOR_GREEN, "+"));
+	ut_assertok(video_osd_print(dev, 0, 4, COLOR_GREEN, "+"));
+
+	/* Add menu caption and entries */
+	ut_assertok(video_osd_print(dev, 5, 0, COLOR_GREEN, " OSD menu "));
+	ut_assertok(video_osd_print(dev, 2, 1, COLOR_BLUE, " *  Entry 1"));
+	ut_assertok(video_osd_print(dev, 2, 2, COLOR_BLUE, "(*) Entry 2"));
+	ut_assertok(video_osd_print(dev, 2, 3, COLOR_BLUE, " *  Entry 3"));
+
+	ut_assertok(video_osd_get_mem(dev, mem, memsize));
+	split(mem, memsize / 2, text, colors);
+
+	print_mem(mem, 20, 5);
+
+	ut_assertok(memcmp(text, "+---- OSD menu ----+"
+				 "|  *  Entry 1      |"
+				 "| (*) Entry 2      |"
+				 "|  *  Entry 3      |"
+				 "+------------------+", memsize / 2));
+
+	ut_assertok(memcmp(colors, "gggggggggggggggggggg"
+				   "gkbbbbbbbbbbbkkkkkkg"
+				   "gkbbbbbbbbbbbkkkkkkg"
+				   "gkbbbbbbbbbbbkkkkkkg"
+				   "gggggggggggggggggggg", memsize / 2));
+
+	return 0;
+}
+DM_TEST(dm_test_osd_extended, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
--
2.11.0

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

* [U-Boot] [PATCH v2 4/5] lib: Import hexdump.c from Linux
  2018-05-23 12:09 [U-Boot] [PATCH v2 1/5] drivers: Add OSD uclass Mario Six
  2018-05-23 12:09 ` [U-Boot] [PATCH v2 2/5] video_osd: Add ihs_video_out driver Mario Six
  2018-05-23 12:09 ` [U-Boot] [PATCH v2 3/5] video_osd: Add osd sandbox driver and tests Mario Six
@ 2018-05-23 12:09 ` Mario Six
  2018-05-23 16:33   ` Simon Glass
  2018-06-04 16:05   ` Alexey Brodkin
  2018-05-23 12:09 ` [U-Boot] [PATCH v2 5/5] cmd: Add osd commands Mario Six
  2018-05-23 16:33 ` [U-Boot] [PATCH v2 1/5] drivers: Add OSD uclass Simon Glass
  4 siblings, 2 replies; 18+ messages in thread
From: Mario Six @ 2018-05-23 12:09 UTC (permalink / raw)
  To: u-boot

Especially for commands, it is useful to be able to turn a hexadecimal
string into its binary representation.

Hence, import the hex_to_bin, bin2hex, and hex2bin functions from the
Linux kernel.

Signed-off-by: Mario Six <mario.six@gdsys.cc>

---

v1 -> v2:
New in v2

---
 include/linux/kernel.h |   4 +
 lib/Makefile           |   1 +
 lib/hexdump.c          | 321 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 326 insertions(+)
 create mode 100644 lib/hexdump.c

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 04a09eb4f64..ea31759667c 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -150,6 +150,10 @@
 		(__x < 0) ? -__x : __x;		\
 	})

+extern int hex_to_bin(char ch);
+extern int __must_check hex2bin(u8 *dst, const char *src, size_t count);
+extern char *bin2hex(char *dst, const void *src, size_t count);
+
 /*
  * min()/max()/clamp() macros that also do
  * strict type-checking.. See the
diff --git a/lib/Makefile b/lib/Makefile
index d531ea54b31..0f6d744579f 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_FIT) += fdtdec_common.o
 obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o
 obj-$(CONFIG_GZIP_COMPRESSED) += gzip.o
 obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += smbios.o
+obj-y += hexdump.o
 obj-y += initcall.o
 obj-$(CONFIG_LMB) += lmb.o
 obj-y += ldiv.o
diff --git a/lib/hexdump.c b/lib/hexdump.c
new file mode 100644
index 00000000000..81587c2ee75
--- /dev/null
+++ b/lib/hexdump.c
@@ -0,0 +1,321 @@
+/*
+ * lib/hexdump.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation. See README and COPYING for
+ * more details.
+ */
+
+#include <linux/compat.h>
+#include <linux/types.h>
+#include <linux/ctype.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/log2.h>
+#include <asm/unaligned.h>
+#include <vsprintf.h>
+
+const char hex_asc[] = "0123456789abcdef";
+EXPORT_SYMBOL(hex_asc);
+const char hex_asc_upper[] = "0123456789ABCDEF";
+EXPORT_SYMBOL(hex_asc_upper);
+
+#define hex_asc_lo(x)	hex_asc[((x) & 0x0f)]
+#define hex_asc_hi(x)	hex_asc[((x) & 0xf0) >> 4]
+
+static inline char *hex_byte_pack(char *buf, u8 byte)
+{
+	*buf++ = hex_asc_hi(byte);
+	*buf++ = hex_asc_lo(byte);
+	return buf;
+}
+
+extern const char hex_asc_upper[];
+#define hex_asc_upper_lo(x)	hex_asc_upper[((x) & 0x0f)]
+#define hex_asc_upper_hi(x)	hex_asc_upper[((x) & 0xf0) >> 4]
+
+static inline char *hex_byte_pack_upper(char *buf, u8 byte)
+{
+	*buf++ = hex_asc_upper_hi(byte);
+	*buf++ = hex_asc_upper_lo(byte);
+	return buf;
+}
+
+/**
+ * hex_to_bin - convert a hex digit to its real value
+ * @ch: ascii character represents hex digit
+ *
+ * hex_to_bin() converts one hex digit to its actual value or -1 in case of bad
+ * input.
+ */
+int hex_to_bin(char ch)
+{
+	if ((ch >= '0') && (ch <= '9'))
+		return ch - '0';
+	ch = tolower(ch);
+	if ((ch >= 'a') && (ch <= 'f'))
+		return ch - 'a' + 10;
+	return -1;
+}
+EXPORT_SYMBOL(hex_to_bin);
+
+/**
+ * hex2bin - convert an ascii hexadecimal string to its binary representation
+ * @dst: binary result
+ * @src: ascii hexadecimal string
+ * @count: result length
+ *
+ * Return 0 on success, -EINVAL in case of bad input.
+ */
+int hex2bin(u8 *dst, const char *src, size_t count)
+{
+	while (count--) {
+		int hi = hex_to_bin(*src++);
+		int lo = hex_to_bin(*src++);
+
+		if ((hi < 0) || (lo < 0))
+			return -EINVAL;
+
+		*dst++ = (hi << 4) | lo;
+	}
+	return 0;
+}
+EXPORT_SYMBOL(hex2bin);
+
+/**
+ * bin2hex - convert binary data to an ascii hexadecimal string
+ * @dst: ascii hexadecimal result
+ * @src: binary data
+ * @count: binary data length
+ */
+char *bin2hex(char *dst, const void *src, size_t count)
+{
+	const unsigned char *_src = src;
+
+	while (count--)
+		dst = hex_byte_pack(dst, *_src++);
+	return dst;
+}
+EXPORT_SYMBOL(bin2hex);
+
+/**
+ * hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory
+ * @buf: data blob to dump
+ * @len: number of bytes in the @buf
+ * @rowsize: number of bytes to print per line; must be 16 or 32
+ * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
+ * @linebuf: where to put the converted data
+ * @linebuflen: total size of @linebuf, including space for terminating NUL
+ * @ascii: include ASCII after the hex output
+ *
+ * hex_dump_to_buffer() works on one "line" of output at a time, i.e.,
+ * 16 or 32 bytes of input data converted to hex + ASCII output.
+ *
+ * Given a buffer of u8 data, hex_dump_to_buffer() converts the input data
+ * to a hex + ASCII dump@the supplied memory location.
+ * The converted output is always NUL-terminated.
+ *
+ * E.g.:
+ *   hex_dump_to_buffer(frame->data, frame->len, 16, 1,
+ *			linebuf, sizeof(linebuf), true);
+ *
+ * example output buffer:
+ * 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f  @ABCDEFGHIJKLMNO
+ *
+ * Return:
+ * The amount of bytes placed in the buffer without terminating NUL. If the
+ * output was truncated, then the return value is the number of bytes
+ * (excluding the terminating NUL) which would have been written to the final
+ * string if enough space had been available.
+ */
+int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
+		       char *linebuf, size_t linebuflen, bool ascii)
+{
+	const u8 *ptr = buf;
+	int ngroups;
+	u8 ch;
+	int j, lx = 0;
+	int ascii_column;
+	int ret;
+
+	if (rowsize != 16 && rowsize != 32)
+		rowsize = 16;
+
+	if (len > rowsize)		/* limit to one line at a time */
+		len = rowsize;
+	if (!is_power_of_2(groupsize) || groupsize > 8)
+		groupsize = 1;
+	if ((len % groupsize) != 0)	/* no mixed size output */
+		groupsize = 1;
+
+	ngroups = len / groupsize;
+	ascii_column = rowsize * 2 + rowsize / groupsize + 1;
+
+	if (!linebuflen)
+		goto overflow1;
+
+	if (!len)
+		goto nil;
+
+	if (groupsize == 8) {
+		const u64 *ptr8 = buf;
+
+		for (j = 0; j < ngroups; j++) {
+			ret = snprintf(linebuf + lx, linebuflen - lx,
+				       "%s%16.16llx", j ? " " : "",
+				       get_unaligned(ptr8 + j));
+			if (ret >= linebuflen - lx)
+				goto overflow1;
+			lx += ret;
+		}
+	} else if (groupsize == 4) {
+		const u32 *ptr4 = buf;
+
+		for (j = 0; j < ngroups; j++) {
+			ret = snprintf(linebuf + lx, linebuflen - lx,
+				       "%s%8.8x", j ? " " : "",
+				       get_unaligned(ptr4 + j));
+			if (ret >= linebuflen - lx)
+				goto overflow1;
+			lx += ret;
+		}
+	} else if (groupsize == 2) {
+		const u16 *ptr2 = buf;
+
+		for (j = 0; j < ngroups; j++) {
+			ret = snprintf(linebuf + lx, linebuflen - lx,
+				       "%s%4.4x", j ? " " : "",
+				       get_unaligned(ptr2 + j));
+			if (ret >= linebuflen - lx)
+				goto overflow1;
+			lx += ret;
+		}
+	} else {
+		for (j = 0; j < len; j++) {
+			if (linebuflen < lx + 2)
+				goto overflow2;
+			ch = ptr[j];
+			linebuf[lx++] = hex_asc_hi(ch);
+			if (linebuflen < lx + 2)
+				goto overflow2;
+			linebuf[lx++] = hex_asc_lo(ch);
+			if (linebuflen < lx + 2)
+				goto overflow2;
+			linebuf[lx++] = ' ';
+		}
+		if (j)
+			lx--;
+	}
+	if (!ascii)
+		goto nil;
+
+	while (lx < ascii_column) {
+		if (linebuflen < lx + 2)
+			goto overflow2;
+		linebuf[lx++] = ' ';
+	}
+	for (j = 0; j < len; j++) {
+		if (linebuflen < lx + 2)
+			goto overflow2;
+		ch = ptr[j];
+		linebuf[lx++] = (isascii(ch) && isprint(ch)) ? ch : '.';
+	}
+nil:
+	linebuf[lx] = '\0';
+	return lx;
+overflow2:
+	linebuf[lx++] = '\0';
+overflow1:
+	return ascii ? ascii_column + len : (groupsize * 2 + 1) * ngroups - 1;
+}
+EXPORT_SYMBOL(hex_dump_to_buffer);
+
+#ifdef CONFIG_PRINTK
+/**
+ * print_hex_dump - print a text hex dump to syslog for a binary blob of data
+ * @level: kernel log level (e.g. KERN_DEBUG)
+ * @prefix_str: string to prefix each line with;
+ *  caller supplies trailing spaces for alignment if desired
+ * @prefix_type: controls whether prefix of an offset, address, or none
+ *  is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE)
+ * @rowsize: number of bytes to print per line; must be 16 or 32
+ * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
+ * @buf: data blob to dump
+ * @len: number of bytes in the @buf
+ * @ascii: include ASCII after the hex output
+ *
+ * Given a buffer of u8 data, print_hex_dump() prints a hex + ASCII dump
+ * to the kernel log at the specified kernel log level, with an optional
+ * leading prefix.
+ *
+ * print_hex_dump() works on one "line" of output@a time, i.e.,
+ * 16 or 32 bytes of input data converted to hex + ASCII output.
+ * print_hex_dump() iterates over the entire input @buf, breaking it into
+ * "line size" chunks to format and print.
+ *
+ * E.g.:
+ *   print_hex_dump(KERN_DEBUG, "raw data: ", DUMP_PREFIX_ADDRESS,
+ *		    16, 1, frame->data, frame->len, true);
+ *
+ * Example output using %DUMP_PREFIX_OFFSET and 1-byte mode:
+ * 0009ab42: 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f  @ABCDEFGHIJKLMNO
+ * Example output using %DUMP_PREFIX_ADDRESS and 4-byte mode:
+ * ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c  pqrstuvwxyz{|}~.
+ */
+void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
+		    int rowsize, int groupsize,
+		    const void *buf, size_t len, bool ascii)
+{
+	const u8 *ptr = buf;
+	int i, linelen, remaining = len;
+	unsigned char linebuf[32 * 3 + 2 + 32 + 1];
+
+	if (rowsize != 16 && rowsize != 32)
+		rowsize = 16;
+
+	for (i = 0; i < len; i += rowsize) {
+		linelen = min(remaining, rowsize);
+		remaining -= rowsize;
+
+		hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
+				   linebuf, sizeof(linebuf), ascii);
+
+		switch (prefix_type) {
+		case DUMP_PREFIX_ADDRESS:
+			printk("%s%s%p: %s\n",
+			       level, prefix_str, ptr + i, linebuf);
+			break;
+		case DUMP_PREFIX_OFFSET:
+			printk("%s%s%.8x: %s\n", level, prefix_str, i, linebuf);
+			break;
+		default:
+			printk("%s%s%s\n", level, prefix_str, linebuf);
+			break;
+		}
+	}
+}
+EXPORT_SYMBOL(print_hex_dump);
+
+#if !defined(CONFIG_DYNAMIC_DEBUG)
+/**
+ * print_hex_dump_bytes - shorthand form of print_hex_dump() with default params
+ * @prefix_str: string to prefix each line with;
+ *  caller supplies trailing spaces for alignment if desired
+ * @prefix_type: controls whether prefix of an offset, address, or none
+ *  is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE)
+ * @buf: data blob to dump
+ * @len: number of bytes in the @buf
+ *
+ * Calls print_hex_dump(), with log level of KERN_DEBUG,
+ * rowsize of 16, groupsize of 1, and ASCII output included.
+ */
+void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
+			  const void *buf, size_t len)
+{
+	print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, 16, 1,
+		       buf, len, true);
+}
+EXPORT_SYMBOL(print_hex_dump_bytes);
+#endif /* !defined(CONFIG_DYNAMIC_DEBUG) */
+#endif /* defined(CONFIG_PRINTK) */
--
2.11.0

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

* [U-Boot] [PATCH v2 5/5] cmd: Add osd commands
  2018-05-23 12:09 [U-Boot] [PATCH v2 1/5] drivers: Add OSD uclass Mario Six
                   ` (2 preceding siblings ...)
  2018-05-23 12:09 ` [U-Boot] [PATCH v2 4/5] lib: Import hexdump.c from Linux Mario Six
@ 2018-05-23 12:09 ` Mario Six
  2018-05-23 16:33   ` Simon Glass
  2018-05-23 16:33 ` [U-Boot] [PATCH v2 1/5] drivers: Add OSD uclass Simon Glass
  4 siblings, 1 reply; 18+ messages in thread
From: Mario Six @ 2018-05-23 12:09 UTC (permalink / raw)
  To: u-boot

Add command to query information from and write text to on-screen
display (OSD) devices.

Signed-off-by: Mario Six <mario.six@gdsys.cc>

---

v1 -> v2:
* Added explanation for what a OSD is
* Added explanation of the color parameter
* Moved GDSYS_LEGACY_OSD_CMDS to gdsys Kconfig

---
 board/gdsys/mpc8308/Kconfig |  11 ++
 cmd/Kconfig                 |   8 +
 cmd/Makefile                |   1 +
 cmd/osd.c                   | 378 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 398 insertions(+)
 create mode 100644 cmd/osd.c

diff --git a/board/gdsys/mpc8308/Kconfig b/board/gdsys/mpc8308/Kconfig
index cb29c25c650..9d99f686923 100644
--- a/board/gdsys/mpc8308/Kconfig
+++ b/board/gdsys/mpc8308/Kconfig
@@ -1,3 +1,9 @@
+config GDSYS_LEGACY_OSD_CMDS
+	bool
+	help
+	  Use the 'osdw', 'osdp', and 'osdsize' legacy commands required by
+	  gdsys devices.
+
 if TARGET_HRCON

 config SYS_BOARD
@@ -9,6 +15,9 @@ config SYS_VENDOR
 config SYS_CONFIG_NAME
 	default "hrcon"

+config GDSYS_LEGACY_OSD_CMDS
+	default y
+
 endif

 if TARGET_STRIDER
@@ -22,6 +31,8 @@ config SYS_VENDOR
 config SYS_CONFIG_NAME
 	default "strider"

+config GDSYS_LEGACY_OSD_CMDS
+	default y
 endif

 config CMD_IOLOOP
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 38406fcfdac..898839c8d75 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -865,6 +865,14 @@ config CMD_ONENAND
 	  and erasing blocks. It allso provides a way to show and change
 	  bad blocks, and test the device.

+config CMD_OSD
+	bool "osd"
+	help
+	  Enable the 'osd' command which allows to query information from and
+	  write text data to a on-screen display (OSD) device; a virtual device
+	  associated with a display capable of displaying a text overlay on the
+	  display it's associated with..
+
 config CMD_PART
 	bool "part"
 	select PARTITION_UUIDS
diff --git a/cmd/Makefile b/cmd/Makefile
index 0d7322ee0a4..4aeab8e5c77 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -62,6 +62,7 @@ obj-$(CONFIG_CMD_FUSE) += fuse.o
 obj-$(CONFIG_CMD_GETTIME) += gettime.o
 obj-$(CONFIG_CMD_GPIO) += gpio.o
 obj-$(CONFIG_CMD_HVC) += smccc.o
+obj-$(CONFIG_CMD_OSD) += osd.o
 obj-$(CONFIG_CMD_I2C) += i2c.o
 obj-$(CONFIG_CMD_IOTRACE) += iotrace.o
 obj-$(CONFIG_CMD_HASH) += hash.o
diff --git a/cmd/osd.c b/cmd/osd.c
new file mode 100644
index 00000000000..8afbe85475b
--- /dev/null
+++ b/cmd/osd.c
@@ -0,0 +1,378 @@
+/*
+ * (C) Copyright 2017
+ * Mario Six,  Guntermann & Drunck GmbH, mario.six at gdsys.cc
+ *
+ * based on the gdsys osd driver, which is
+ *
+ * (C) Copyright 2010
+ * Dirk Eibach,  Guntermann & Drunck GmbH, eibach at gdsys.de
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <video_osd.h>
+#include <malloc.h>
+
+#ifndef CONFIG_GDSYS_LEGACY_OSD_CMDS
+static struct udevice *osd_cur;
+#endif
+
+#ifdef CONFIG_GDSYS_LEGACY_OSD_CMDS
+int do_osd_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	struct udevice *dev;
+	uint x, y;
+	uint count;
+	char *hexstr;
+	u8 *buffer;
+	size_t buflen;
+
+	if (argc < 4 || (strlen(argv[3])) % 2) {
+		cmd_usage(cmdtp);
+		return CMD_RET_FAILURE;
+	}
+
+	x = simple_strtoul(argv[1], NULL, 16);
+	y = simple_strtoul(argv[2], NULL, 16);
+	hexstr = argv[3];
+	count = (argc > 4) ? simple_strtoul(argv[4], NULL, 16) : 1;
+
+	buflen = strlen(hexstr) / 2;
+	buffer = malloc(buflen);
+	hex2bin(buffer, hexstr, buflen);
+
+	for (uclass_first_device(UCLASS_VIDEO_OSD, &dev);
+	     dev;
+	     uclass_next_device(&dev)) {
+		int res;
+
+		res = video_osd_set_mem(dev, x, y, buffer, buflen, count);
+
+		if (res) {
+			printf("Could not write to video mem on osd %s\n",
+			       dev->name);
+			return CMD_RET_FAILURE;
+		}
+	}
+
+	free(buffer);
+
+	return CMD_RET_SUCCESS;
+}
+
+static int do_osd_print(cmd_tbl_t *cmdtp, int flag, int argc,
+			char * const argv[])
+{
+	struct udevice *dev;
+	uint x, y;
+	u8 color;
+	char *text;
+
+	if (argc < 5) {
+		cmd_usage(cmdtp);
+		return CMD_RET_FAILURE;
+	}
+
+	x = simple_strtoul(argv[1], NULL, 16);
+	y = simple_strtoul(argv[2], NULL, 16);
+	color = simple_strtoul(argv[3], NULL, 16);
+	text = argv[4];
+
+	for (uclass_first_device(UCLASS_VIDEO_OSD, &dev);
+	     dev;
+	     uclass_next_device(&dev)) {
+		int res;
+
+		res = video_osd_print(dev, x, y, color, text);
+		if (res) {
+			printf("Could not print string to osd %s\n", dev->name);
+			return CMD_RET_FAILURE;
+		}
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
+int do_osd_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	struct udevice *dev;
+	uint x, y;
+
+	if (argc < 3) {
+		cmd_usage(cmdtp);
+		return CMD_RET_FAILURE;
+	}
+
+	x = simple_strtoul(argv[1], NULL, 16);
+	y = simple_strtoul(argv[2], NULL, 16);
+
+	for (uclass_first_device(UCLASS_VIDEO_OSD, &dev);
+	     dev;
+	     uclass_next_device(&dev)) {
+		int res;
+		res = video_osd_set_size(dev, x, y);
+
+		if (res) {
+			printf("Could not set size on osd %s\n", dev->name);
+			return CMD_RET_FAILURE;
+		}
+	}
+
+	return CMD_RET_SUCCESS;
+}
+#else
+int do_osd_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	uint x, y;
+	uint count;
+	char *hexstr;
+	u8 *buffer;
+	size_t buflen;
+	int res;
+
+	if (argc < 4 || (strlen(argv[3]) % 2)) {
+		cmd_usage(cmdtp);
+		return CMD_RET_FAILURE;
+	}
+
+	if (!osd_cur) {
+		puts("No osd selected\n");
+		return CMD_RET_FAILURE;
+	}
+
+	x = simple_strtoul(argv[1], NULL, 16);
+	y = simple_strtoul(argv[2], NULL, 16);
+	hexstr = argv[3];
+	count = (argc > 4) ? simple_strtoul(argv[4], NULL, 16) : 1;
+
+	buflen = strlen(hexstr) / 2;
+	buffer = malloc(buflen);
+	hex2bin(buffer, hexstr, buflen);
+
+	res = video_osd_set_mem(osd_cur, x, y, buffer, buflen, count);
+
+	if (res) {
+		printf("Could not write to video mem on osd %s\n",
+		       osd_cur->name);
+		return CMD_RET_FAILURE;
+	}
+
+	free(buffer);
+
+	return CMD_RET_SUCCESS;
+}
+
+int do_osd_print(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	uint x, y;
+	u8 color;
+	char *text;
+	int res;
+
+	if (argc < 5) {
+		cmd_usage(cmdtp);
+		return CMD_RET_FAILURE;
+	}
+
+	if (!osd_cur) {
+		puts("No osd selected\n");
+		return CMD_RET_FAILURE;
+	}
+
+	x = simple_strtoul(argv[1], NULL, 16);
+	y = simple_strtoul(argv[2], NULL, 16);
+	color = simple_strtoul(argv[3], NULL, 16);
+	text = argv[4];
+
+	res = video_osd_print(osd_cur, x, y, color, text);
+	if (res) {
+		printf("Could not print string to osd %s\n", osd_cur->name);
+		return CMD_RET_FAILURE;
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
+int do_osd_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	uint x, y;
+	int res;
+
+	if (argc < 3) {
+		cmd_usage(cmdtp);
+		return CMD_RET_FAILURE;
+	}
+
+	if (!osd_cur) {
+		puts("No osd selected\n");
+		return CMD_RET_FAILURE;
+	}
+
+	x = simple_strtoul(argv[1], NULL, 16);
+	y = simple_strtoul(argv[2], NULL, 16);
+
+	res = video_osd_set_size(osd_cur, x, y);
+	if (res) {
+		printf("Could not set size on osd %s\n", osd_cur->name);
+		return CMD_RET_FAILURE;
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
+static void show_osd(struct udevice *osd)
+{
+	printf("OSD %d:\t%s", osd->req_seq, osd->name);
+	if (device_active(osd))
+		printf("  (active %d)", osd->seq);
+	printf("\n");
+}
+
+static int do_show_osd(cmd_tbl_t *cmdtp, int flag, int argc,
+		       char * const argv[])
+{
+	struct udevice *osd;
+
+	if (argc == 1) {
+		/* show all OSDs */
+		struct uclass *uc;
+		int ret;
+
+		ret = uclass_get(UCLASS_VIDEO_OSD, &uc);
+		if (ret)
+			return CMD_RET_FAILURE;
+		uclass_foreach_dev(osd, uc)
+			show_osd(osd);
+	} else {
+		int i, ret;
+
+		/* show specific OSD */
+		i = simple_strtoul(argv[1], NULL, 10);
+
+		ret = uclass_get_device_by_seq(UCLASS_VIDEO_OSD, i, &osd);
+		if (ret) {
+			printf("Invalid osd %d: err=%d\n", i, ret);
+			return CMD_RET_FAILURE;
+		}
+		show_osd(osd);
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
+static int cmd_osd_set_osd_num(unsigned int osdnum)
+{
+	struct udevice *osd;
+	int ret;
+
+	ret = uclass_get_device_by_seq(UCLASS_VIDEO_OSD, osdnum, &osd);
+	if (ret) {
+		printf("%s: No OSD %d\n", __func__, osdnum);
+		return ret;
+	}
+	osd_cur = osd;
+
+	return 0;
+}
+
+static int osd_get_osd_cur(struct udevice **osdp)
+{
+	if (!osd_cur) {
+		puts("No osd selected\n");
+		return -ENODEV;
+	}
+	*osdp = osd_cur;
+
+	return 0;
+}
+
+static int do_osd_num(cmd_tbl_t *cmdtp, int flag, int argc,
+		      char * const argv[])
+{
+	int ret = 0;
+	int osd_no;
+
+	if (argc == 1) {
+		/* querying current setting */
+		struct udevice *osd;
+
+		if (!osd_get_osd_cur(&osd))
+			osd_no = osd->seq;
+		else
+			osd_no = -1;
+		printf("Current osd is %d\n", osd_no);
+	} else {
+		osd_no = simple_strtoul(argv[1], NULL, 10);
+		printf("Setting osd to %d\n", osd_no);
+
+		ret = cmd_osd_set_osd_num(osd_no);
+
+		if (ret)
+			printf("Failure changing osd number (%d)\n", ret);
+	}
+
+	return ret ? CMD_RET_FAILURE : CMD_RET_SUCCESS;
+}
+
+static cmd_tbl_t cmd_osd_sub[] = {
+	U_BOOT_CMD_MKENT(show, 1, 1, do_show_osd, "", ""),
+	U_BOOT_CMD_MKENT(dev, 1, 1, do_osd_num, "", ""),
+	U_BOOT_CMD_MKENT(write, 4, 1, do_osd_write, "", ""),
+	U_BOOT_CMD_MKENT(print, 4, 1, do_osd_print, "", ""),
+	U_BOOT_CMD_MKENT(size, 2, 1, do_osd_size, "", ""),
+};
+
+static int do_osd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	cmd_tbl_t *c;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	/* Strip off leading 'osd' command argument */
+	argc--;
+	argv++;
+
+	c = find_cmd_tbl(argv[0], &cmd_osd_sub[0], ARRAY_SIZE(cmd_osd_sub));
+
+	if (c)
+		return c->cmd(cmdtp, flag, argc, argv);
+	else
+		return CMD_RET_USAGE;
+}
+#endif
+
+#ifdef CONFIG_GDSYS_LEGACY_OSD_CMDS
+U_BOOT_CMD(
+	osdw, 5, 0, do_osd_write,
+	"write 16-bit hex encoded buffer to osd memory",
+	"osdw [pos_x] [pos_y] [buffer] [count] - write 8-bit hex encoded buffer to osd memory\n"
+);
+
+U_BOOT_CMD(
+	osdp, 5, 0, do_osd_print,
+	"write ASCII buffer to osd memory",
+	"osdp [pos_x] [pos_y] [color] [text] - write ASCII buffer to osd memory\n"
+);
+
+U_BOOT_CMD(
+	osdsize, 3, 0, do_osd_size,
+	"set OSD XY size in characters",
+	"osdsize [size_x] [size_y] - set OSD XY size in characters\n"
+);
+#else
+static char osd_help_text[] =
+	"show  - show OSD info\n"
+	"osd dev [dev] - show or set current OSD\n"
+	"write [pos_x] [pos_y] [buffer] [count] - write 8-bit hex encoded buffer to osd memory at a given position\n"
+	"print [pos_x] [pos_y] [color] [text] - write ASCII buffer (given by text data and driver-specific color information) to osd memory\n"
+	"size [size_x] [size_y] - set OSD XY size in characters\n";
+
+U_BOOT_CMD(
+	osd, 6, 1, do_osd,
+	"OSD sub-system",
+	osd_help_text
+);
+#endif
--
2.11.0

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

* [U-Boot] [PATCH v2 3/5] video_osd: Add osd sandbox driver and tests
  2018-05-23 12:09 ` [U-Boot] [PATCH v2 3/5] video_osd: Add osd sandbox driver and tests Mario Six
@ 2018-05-23 16:33   ` Simon Glass
  2018-05-25 11:19     ` Mario Six
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2018-05-23 16:33 UTC (permalink / raw)
  To: u-boot

Hi Mario,

On 23 May 2018 at 06:09, Mario Six <mario.six@gdsys.cc> wrote:
> Add sandbox driver and tests for the new OSD uclass.
>
> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>
> ---
>
> v1 -> v2:
> New in v2
>
> ---
>  arch/sandbox/dts/test.dts          |   4 +
>  configs/sandbox64_defconfig        |   3 +
>  configs/sandbox_defconfig          |   3 +
>  configs/sandbox_flattree_defconfig |   3 +
>  configs/sandbox_noblk_defconfig    |   3 +
>  configs/sandbox_spl_defconfig      |   3 +
>  drivers/video/Kconfig              |   6 ++
>  drivers/video/Makefile             |   1 +
>  drivers/video/sandbox_osd.c        | 157 ++++++++++++++++++++++++++++
>  drivers/video/sandbox_osd.h        |  14 +++
>  drivers/video/video_osd-uclass.c   |   9 ++
>  include/video_osd.h                |   7 ++
>  test/dm/Makefile                   |   1 +
>  test/dm/osd.c                      | 208 +++++++++++++++++++++++++++++++++++++
>  14 files changed, 422 insertions(+)
>  create mode 100644 drivers/video/sandbox_osd.c
>  create mode 100644 drivers/video/sandbox_osd.h
>  create mode 100644 test/dm/osd.c

This looks good. But you can't add a new get_mem() operation just for sandbox.

Instead, how about a back-door function that allows sandbox to get its
information. You can call it sandbox_video_osd_get_mem(), for example,
and just directly call it from you test code and implement it in your
driver.

There are some functions like this in arch/sandbox/include/asm/test.h

Regards,
Simon

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

* [U-Boot] [PATCH v2 1/5] drivers: Add OSD uclass
  2018-05-23 12:09 [U-Boot] [PATCH v2 1/5] drivers: Add OSD uclass Mario Six
                   ` (3 preceding siblings ...)
  2018-05-23 12:09 ` [U-Boot] [PATCH v2 5/5] cmd: Add osd commands Mario Six
@ 2018-05-23 16:33 ` Simon Glass
  2018-05-25 11:25   ` Mario Six
  4 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2018-05-23 16:33 UTC (permalink / raw)
  To: u-boot

On 23 May 2018 at 06:09, Mario Six <mario.six@gdsys.cc> wrote:
> Some devices offer a text-based OSD (on-screen display) that can be
> programmatically controlled (i.e. text displayed on).
>
> Add a uclass to support such devices.
>
> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>
> ---
>
> v1 -> v2:
> * Use singular case for UCLASS_VIDEO_OSD description
> * Expanded description of video_osd_set_mem with example
> * Replaced all left-over mentions of pixels with text
> * Renamed x and y parameters to col and row
> * Expaneded description of color parameter
> * Added general description of the OSD uclass
> * Expanded description of video_osd_info
>
> ---
>  drivers/video/Kconfig            |   8 ++
>  drivers/video/Makefile           |   2 +
>  drivers/video/video_osd-uclass.c |  46 ++++++++++
>  include/dm/uclass-id.h           |   1 +
>  include/video_osd.h              | 193 +++++++++++++++++++++++++++++++++++++++
>  5 files changed, 250 insertions(+)
>  create mode 100644 drivers/video/video_osd-uclass.c
>  create mode 100644 include/video_osd.h

Why drop the 'u' on 'colour'?

Reviewed-by: Simon Glass <sjg@chromium.org>

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

* [U-Boot] [PATCH v2 2/5] video_osd: Add ihs_video_out driver
  2018-05-23 12:09 ` [U-Boot] [PATCH v2 2/5] video_osd: Add ihs_video_out driver Mario Six
@ 2018-05-23 16:33   ` Simon Glass
  2018-05-25 11:20     ` Mario Six
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2018-05-23 16:33 UTC (permalink / raw)
  To: u-boot

Hi Mario,

On 23 May 2018 at 06:09, Mario Six <mario.six@gdsys.cc> wrote:
> Add a driver for IHS OSDs on IHS FPGAs.
>
> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>
> ---
>
> v1 -> v2:
> * Renamed x and y parameters to col and row
> * Removed duplicate IHS_VIDEO_OUT definition
> * Switched error return code of ihs_video_out_set_mem to E2BIG
> * Dropped blank lines between dev_read_u32_default calls
> * Documented members of ihs_video_out_priv
> * Turned printfs into debugs
> * Don't display OSD until something has been written to it
> * Made the code a bit more readable
> * Added binding file
> * Expanded documentation and help
> * Switched to display_enable
> * Switched to regmap usage (instead of fpgamap uclass)
> * Renamed 'dp_tx' property to 'video_tx'
>
> ---
>  .../video/osd/gdsys,ihs_video_out.txt              |  22 ++
>  drivers/video/Kconfig                              |   9 +
>  drivers/video/Makefile                             |   1 +
>  drivers/video/ihs_video_out.c                      | 277 +++++++++++++++++++++
>  4 files changed, 309 insertions(+)
>  create mode 100644 doc/device-tree-bindings/video/osd/gdsys,ihs_video_out.txt
>  create mode 100644 drivers/video/ihs_video_out.c
>

Reviewed-by: Simon Glass <sjg@chromium.org>

You could perhaps s/dev_read_prop/dev_read_string/ to avoid the cast.
Also is there a binding file for your driver's device-tree node
somewhere?

Regards,
Simon

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

* [U-Boot] [PATCH v2 5/5] cmd: Add osd commands
  2018-05-23 12:09 ` [U-Boot] [PATCH v2 5/5] cmd: Add osd commands Mario Six
@ 2018-05-23 16:33   ` Simon Glass
  2018-05-25 11:23     ` Mario Six
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2018-05-23 16:33 UTC (permalink / raw)
  To: u-boot

Hi Mario,

On 23 May 2018 at 06:09, Mario Six <mario.six@gdsys.cc> wrote:
> Add command to query information from and write text to on-screen
> display (OSD) devices.
>
> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>
> ---
>
> v1 -> v2:
> * Added explanation for what a OSD is
> * Added explanation of the color parameter
> * Moved GDSYS_LEGACY_OSD_CMDS to gdsys Kconfig
>
> ---
>  board/gdsys/mpc8308/Kconfig |  11 ++
>  cmd/Kconfig                 |   8 +
>  cmd/Makefile                |   1 +
>  cmd/osd.c                   | 378 ++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 398 insertions(+)
>  create mode 100644 cmd/osd.c

To me it seems unfortunate that the legacy and new commands are named
the same. I suppose that is unavoidable?

I wonder if the legacy code could be moved to board/gdsys instead? It
is out of the way then.

Regards,
Simon

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

* [U-Boot] [PATCH v2 4/5] lib: Import hexdump.c from Linux
  2018-05-23 12:09 ` [U-Boot] [PATCH v2 4/5] lib: Import hexdump.c from Linux Mario Six
@ 2018-05-23 16:33   ` Simon Glass
  2018-05-25 11:22     ` Mario Six
  2018-06-04 16:05   ` Alexey Brodkin
  1 sibling, 1 reply; 18+ messages in thread
From: Simon Glass @ 2018-05-23 16:33 UTC (permalink / raw)
  To: u-boot

Hi Mario,

On 23 May 2018 at 06:09, Mario Six <mario.six@gdsys.cc> wrote:
> Especially for commands, it is useful to be able to turn a hexadecimal
> string into its binary representation.
>
> Hence, import the hex_to_bin, bin2hex, and hex2bin functions from the
> Linux kernel.
>
> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>
> ---
>
> v1 -> v2:
> New in v2
>
> ---
>  include/linux/kernel.h |   4 +
>  lib/Makefile           |   1 +
>  lib/hexdump.c          | 321 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 326 insertions(+)
>  create mode 100644 lib/hexdump.c

Does Linux have any tests for this code?

Regards,
Simon

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

* [U-Boot] [PATCH v2 3/5] video_osd: Add osd sandbox driver and tests
  2018-05-23 16:33   ` Simon Glass
@ 2018-05-25 11:19     ` Mario Six
  0 siblings, 0 replies; 18+ messages in thread
From: Mario Six @ 2018-05-25 11:19 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On Wed, May 23, 2018 at 6:33 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Mario,
>
> On 23 May 2018 at 06:09, Mario Six <mario.six@gdsys.cc> wrote:
>> Add sandbox driver and tests for the new OSD uclass.
>>
>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>>
>> ---
>>
>> v1 -> v2:
>> New in v2
>>
>> ---
>>  arch/sandbox/dts/test.dts          |   4 +
>>  configs/sandbox64_defconfig        |   3 +
>>  configs/sandbox_defconfig          |   3 +
>>  configs/sandbox_flattree_defconfig |   3 +
>>  configs/sandbox_noblk_defconfig    |   3 +
>>  configs/sandbox_spl_defconfig      |   3 +
>>  drivers/video/Kconfig              |   6 ++
>>  drivers/video/Makefile             |   1 +
>>  drivers/video/sandbox_osd.c        | 157 ++++++++++++++++++++++++++++
>>  drivers/video/sandbox_osd.h        |  14 +++
>>  drivers/video/video_osd-uclass.c   |   9 ++
>>  include/video_osd.h                |   7 ++
>>  test/dm/Makefile                   |   1 +
>>  test/dm/osd.c                      | 208 +++++++++++++++++++++++++++++++++++++
>>  14 files changed, 422 insertions(+)
>>  create mode 100644 drivers/video/sandbox_osd.c
>>  create mode 100644 drivers/video/sandbox_osd.h
>>  create mode 100644 test/dm/osd.c
>
> This looks good. But you can't add a new get_mem() operation just for sandbox.
>
> Instead, how about a back-door function that allows sandbox to get its
> information. You can call it sandbox_video_osd_get_mem(), for example,
> and just directly call it from you test code and implement it in your
> driver.
>
> There are some functions like this in arch/sandbox/include/asm/test.h
>

OK, I will do that for v3.

> Regards,
> Simon
>

Best regards,
Mario

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

* [U-Boot] [PATCH v2 2/5] video_osd: Add ihs_video_out driver
  2018-05-23 16:33   ` Simon Glass
@ 2018-05-25 11:20     ` Mario Six
  0 siblings, 0 replies; 18+ messages in thread
From: Mario Six @ 2018-05-25 11:20 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On Wed, May 23, 2018 at 6:33 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Mario,
>
> On 23 May 2018 at 06:09, Mario Six <mario.six@gdsys.cc> wrote:
>> Add a driver for IHS OSDs on IHS FPGAs.
>>
>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>>
>> ---
>>
>> v1 -> v2:
>> * Renamed x and y parameters to col and row
>> * Removed duplicate IHS_VIDEO_OUT definition
>> * Switched error return code of ihs_video_out_set_mem to E2BIG
>> * Dropped blank lines between dev_read_u32_default calls
>> * Documented members of ihs_video_out_priv
>> * Turned printfs into debugs
>> * Don't display OSD until something has been written to it
>> * Made the code a bit more readable
>> * Added binding file
>> * Expanded documentation and help
>> * Switched to display_enable
>> * Switched to regmap usage (instead of fpgamap uclass)
>> * Renamed 'dp_tx' property to 'video_tx'
>>
>> ---
>>  .../video/osd/gdsys,ihs_video_out.txt              |  22 ++
>>  drivers/video/Kconfig                              |   9 +
>>  drivers/video/Makefile                             |   1 +
>>  drivers/video/ihs_video_out.c                      | 277 +++++++++++++++++++++
>>  4 files changed, 309 insertions(+)
>>  create mode 100644 doc/device-tree-bindings/video/osd/gdsys,ihs_video_out.txt
>>  create mode 100644 drivers/video/ihs_video_out.c
>>
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> You could perhaps s/dev_read_prop/dev_read_string/ to avoid the cast.
> Also is there a binding file for your driver's device-tree node
> somewhere?
>

Sure, no problem, and, no, there isn't. I will write a binding file for v3.

> Regards,
> Simon
>

Best regards,
Mario

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

* [U-Boot] [PATCH v2 4/5] lib: Import hexdump.c from Linux
  2018-05-23 16:33   ` Simon Glass
@ 2018-05-25 11:22     ` Mario Six
  0 siblings, 0 replies; 18+ messages in thread
From: Mario Six @ 2018-05-25 11:22 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On Wed, May 23, 2018 at 6:33 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Mario,
>
> On 23 May 2018 at 06:09, Mario Six <mario.six@gdsys.cc> wrote:
>> Especially for commands, it is useful to be able to turn a hexadecimal
>> string into its binary representation.
>>
>> Hence, import the hex_to_bin, bin2hex, and hex2bin functions from the
>> Linux kernel.
>>
>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>>
>> ---
>>
>> v1 -> v2:
>> New in v2
>>
>> ---
>>  include/linux/kernel.h |   4 +
>>  lib/Makefile           |   1 +
>>  lib/hexdump.c          | 321 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 326 insertions(+)
>>  create mode 100644 lib/hexdump.c
>
> Does Linux have any tests for this code?
>

No, there are no tests for this. I'll write some for v3, then.

> Regards,
> Simon
>

Best regards,
Mario

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

* [U-Boot] [PATCH v2 5/5] cmd: Add osd commands
  2018-05-23 16:33   ` Simon Glass
@ 2018-05-25 11:23     ` Mario Six
  0 siblings, 0 replies; 18+ messages in thread
From: Mario Six @ 2018-05-25 11:23 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On Wed, May 23, 2018 at 6:33 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Mario,
>
> On 23 May 2018 at 06:09, Mario Six <mario.six@gdsys.cc> wrote:
>> Add command to query information from and write text to on-screen
>> display (OSD) devices.
>>
>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>>
>> ---
>>
>> v1 -> v2:
>> * Added explanation for what a OSD is
>> * Added explanation of the color parameter
>> * Moved GDSYS_LEGACY_OSD_CMDS to gdsys Kconfig
>>
>> ---
>>  board/gdsys/mpc8308/Kconfig |  11 ++
>>  cmd/Kconfig                 |   8 +
>>  cmd/Makefile                |   1 +
>>  cmd/osd.c                   | 378 ++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 398 insertions(+)
>>  create mode 100644 cmd/osd.c
>
> To me it seems unfortunate that the legacy and new commands are named
> the same. I suppose that is unavoidable?
>
> I wonder if the legacy code could be moved to board/gdsys instead? It
> is out of the way then.
>

Sounds like a good idea. I'll separate the commands for v3.

> Regards,
> Simon
>

Best regards,
Mario

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

* [U-Boot] [PATCH v2 1/5] drivers: Add OSD uclass
  2018-05-23 16:33 ` [U-Boot] [PATCH v2 1/5] drivers: Add OSD uclass Simon Glass
@ 2018-05-25 11:25   ` Mario Six
  0 siblings, 0 replies; 18+ messages in thread
From: Mario Six @ 2018-05-25 11:25 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On Wed, May 23, 2018 at 6:33 PM, Simon Glass <sjg@chromium.org> wrote:
> On 23 May 2018 at 06:09, Mario Six <mario.six@gdsys.cc> wrote:
>> Some devices offer a text-based OSD (on-screen display) that can be
>> programmatically controlled (i.e. text displayed on).
>>
>> Add a uclass to support such devices.
>>
>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>>
>> ---
>>
>> v1 -> v2:
>> * Use singular case for UCLASS_VIDEO_OSD description
>> * Expanded description of video_osd_set_mem with example
>> * Replaced all left-over mentions of pixels with text
>> * Renamed x and y parameters to col and row
>> * Expaneded description of color parameter
>> * Added general description of the OSD uclass
>> * Expanded description of video_osd_info
>>
>> ---
>>  drivers/video/Kconfig            |   8 ++
>>  drivers/video/Makefile           |   2 +
>>  drivers/video/video_osd-uclass.c |  46 ++++++++++
>>  include/dm/uclass-id.h           |   1 +
>>  include/video_osd.h              | 193 +++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 250 insertions(+)
>>  create mode 100644 drivers/video/video_osd-uclass.c
>>  create mode 100644 include/video_osd.h
>
> Why drop the 'u' on 'colour'?
>

# git grep color | wc -l
794

# git grep colour | wc -l
191

;-)

> Reviewed-by: Simon Glass <sjg@chromium.org>
>

Best regards,
Mario

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

* [U-Boot] [PATCH v2 4/5] lib: Import hexdump.c from Linux
  2018-05-23 12:09 ` [U-Boot] [PATCH v2 4/5] lib: Import hexdump.c from Linux Mario Six
  2018-05-23 16:33   ` Simon Glass
@ 2018-06-04 16:05   ` Alexey Brodkin
  2018-06-13  8:24     ` Mario Six
  1 sibling, 1 reply; 18+ messages in thread
From: Alexey Brodkin @ 2018-06-04 16:05 UTC (permalink / raw)
  To: u-boot

Hi Mario,

On Wed, 2018-05-23 at 14:09 +0200, Mario Six wrote:
> Especially for commands, it is useful to be able to turn a hexadecimal
> string into its binary representation.
> 
> Hence, import the hex_to_bin, bin2hex, and hex2bin functions from the
> Linux kernel.
> 
> Signed-off-by: Mario Six <mario.six@gdsys.cc>
> 
> ---
> 
> v1 -> v2:
> New in v2

Something is missing?

Note there was a similar discussion some time ago here:
https://patchwork.ozlabs.org/patch/633733/, might worth checking.

If of any interest you may pick up my earlier patch and do
fix-ups mentioned by Tom:
 1. Move hexdump.h away from common.h
 2. Update existing users of print_hex_dump() in U-Boot
    such that they don't use debug level (i.e. drop the first argument)

Or I may do the same re-spin sometime soon.

Still read-on for a couple of comments for your patch.

[snip]

>  /*
>   * min()/max()/clamp() macros that also do
>   * strict type-checking.. See the
> diff --git a/lib/Makefile b/lib/Makefile
> index d531ea54b31..0f6d744579f 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_FIT) += fdtdec_common.o
>  obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o
>  obj-$(CONFIG_GZIP_COMPRESSED) += gzip.o
>  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += smbios.o
> +obj-y += hexdump.o

U-Boot might be used on targets with limited memory
so having ability to include hexdump or not might be
beneficial here. Especially in production builds why would you need hexdump?

[snip]

> +#ifdef CONFIG_PRINTK

Why PRINTK in U-Boot? It's purely kernel's thing.

> +#if !defined(CONFIG_DYNAMIC_DEBUG)

Ditto, CONFIG_DYNAMIC_DEBUG has nothing to do with U-Boot.

-Alexey

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

* [U-Boot] [PATCH v2 4/5] lib: Import hexdump.c from Linux
  2018-06-04 16:05   ` Alexey Brodkin
@ 2018-06-13  8:24     ` Mario Six
  2018-06-13 16:04       ` Alexey Brodkin
  0 siblings, 1 reply; 18+ messages in thread
From: Mario Six @ 2018-06-13  8:24 UTC (permalink / raw)
  To: u-boot

Hi Alexey,

On Mon, Jun 4, 2018 at 6:05 PM, Alexey Brodkin
<Alexey.Brodkin@synopsys.com> wrote:
> Hi Mario,
>
> On Wed, 2018-05-23 at 14:09 +0200, Mario Six wrote:
>> Especially for commands, it is useful to be able to turn a hexadecimal
>> string into its binary representation.
>>
>> Hence, import the hex_to_bin, bin2hex, and hex2bin functions from the
>> Linux kernel.
>>
>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>>
>> ---
>>
>> v1 -> v2:
>> New in v2
>
> Something is missing?
>
> Note there was a similar discussion some time ago here:
> https://patchwork.ozlabs.org/patch/633733/, might worth checking.
>
> If of any interest you may pick up my earlier patch and do
> fix-ups mentioned by Tom:
>  1. Move hexdump.h away from common.h
>  2. Update existing users of print_hex_dump() in U-Boot
>     such that they don't use debug level (i.e. drop the first argument)
>
> Or I may do the same re-spin sometime soon.
>

Thanks for your feedback! I saw that you posted a re-spin of your patch;
Thank you, that's very helpful.

> Still read-on for a couple of comments for your patch.
>
> [snip]
>
>>  /*
>>   * min()/max()/clamp() macros that also do
>>   * strict type-checking.. See the
>> diff --git a/lib/Makefile b/lib/Makefile
>> index d531ea54b31..0f6d744579f 100644
>> --- a/lib/Makefile
>> +++ b/lib/Makefile
>> @@ -29,6 +29,7 @@ obj-$(CONFIG_FIT) += fdtdec_common.o
>>  obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o
>>  obj-$(CONFIG_GZIP_COMPRESSED) += gzip.o
>>  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += smbios.o
>> +obj-y += hexdump.o
>
> U-Boot might be used on targets with limited memory
> so having ability to include hexdump or not might be
> beneficial here. Especially in production builds why would you need hexdump?
>

Yes, it's probably better to have it deactivatable, true. But as for why
production builds need hexdump: It's not so much the hexdump function, but the
bin2hex function, which can be used in a number of U-Boot commands that read
hexadecimal data. We use one such command to initialize a TPM on one of our
boards, for example.

> [snip]
>
>> +#ifdef CONFIG_PRINTK
>
> Why PRINTK in U-Boot? It's purely kernel's thing.
>
>> +#if !defined(CONFIG_DYNAMIC_DEBUG)
>
> Ditto, CONFIG_DYNAMIC_DEBUG has nothing to do with U-Boot.
>

Those were copied verbatim from the kernel, I'm pretty sure I just got them by
mistake.

> -Alexey
>

Best regards,
Mario

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

* [U-Boot] [PATCH v2 4/5] lib: Import hexdump.c from Linux
  2018-06-13  8:24     ` Mario Six
@ 2018-06-13 16:04       ` Alexey Brodkin
  0 siblings, 0 replies; 18+ messages in thread
From: Alexey Brodkin @ 2018-06-13 16:04 UTC (permalink / raw)
  To: u-boot

On Wed, 2018-06-13 at 10:24 +0200, Mario Six wrote:
> Hi Alexey,
> 
> On Mon, Jun 4, 2018 at 6:05 PM, Alexey Brodkin
> <Alexey.Brodkin@synopsys.com> wrote:
> > Hi Mario,
> > 
> > On Wed, 2018-05-23 at 14:09 +0200, Mario Six wrote:
> > > Especially for commands, it is useful to be able to turn a hexadecimal
> > > string into its binary representation.
> > > 
> > > Hence, import the hex_to_bin, bin2hex, and hex2bin functions from the
> > > Linux kernel.
> > > 
> > > Signed-off-by: Mario Six <mario.six@gdsys.cc>
> > > 
> > > ---
> > > 
> > > v1 -> v2:
> > > New in v2
> > 
> > Something is missing?
> > 
> > Note there was a similar discussion some time ago here:
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_633733_&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=lqdeeSSEes0GFDDl656eVi
> > XO7breS55ytWkhpk5R81I&m=mnw00As1T_NpnFNncTURuK4YkIAYD-Dj-VTNkRqquVY&s=HcgjkTO1GdtGqH9sYIjizH30AC3m_Xfa1F6n_Cy_qZY&e=, might worth checking.
> > 
> > If of any interest you may pick up my earlier patch and do
> > fix-ups mentioned by Tom:
> >  1. Move hexdump.h away from common.h
> >  2. Update existing users of print_hex_dump() in U-Boot
> >     such that they don't use debug level (i.e. drop the first argument)
> > 
> > Or I may do the same re-spin sometime soon.
> > 
> 
> Thanks for your feedback! I saw that you posted a re-spin of your patch;
> Thank you, that's very helpful.

FWIW Tom just pulled that in, see
http://git.denx.de/?p=u-boot.git;a=commit;h=f8c987f8f127f867d96ca74bcd1fcb11d8265b67

> 
> > Still read-on for a couple of comments for your patch.
> > 
> > [snip]
> > 
> > >  /*
> > >   * min()/max()/clamp() macros that also do
> > >   * strict type-checking.. See the
> > > diff --git a/lib/Makefile b/lib/Makefile
> > > index d531ea54b31..0f6d744579f 100644
> > > --- a/lib/Makefile
> > > +++ b/lib/Makefile
> > > @@ -29,6 +29,7 @@ obj-$(CONFIG_FIT) += fdtdec_common.o
> > >  obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o
> > >  obj-$(CONFIG_GZIP_COMPRESSED) += gzip.o
> > >  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += smbios.o
> > > +obj-y += hexdump.o
> > 
> > U-Boot might be used on targets with limited memory
> > so having ability to include hexdump or not might be
> > beneficial here. Especially in production builds why would you need hexdump?
> > 
> 
> Yes, it's probably better to have it deactivatable, true. But as for why
> production builds need hexdump: It's not so much the hexdump function, but the
> bin2hex function, which can be used in a number of U-Boot commands that read
> hexadecimal data. We use one such command to initialize a TPM on one of our
> boards, for example.

I spoke a bit too soon as I forgot what I did before :)
We don't disable building of entire hexdump.o instead in hexdump.c we
just put some code in #ifdefs that way you may move bin2hex() outside
#ifdef.

-Alexey

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

end of thread, other threads:[~2018-06-13 16:04 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-23 12:09 [U-Boot] [PATCH v2 1/5] drivers: Add OSD uclass Mario Six
2018-05-23 12:09 ` [U-Boot] [PATCH v2 2/5] video_osd: Add ihs_video_out driver Mario Six
2018-05-23 16:33   ` Simon Glass
2018-05-25 11:20     ` Mario Six
2018-05-23 12:09 ` [U-Boot] [PATCH v2 3/5] video_osd: Add osd sandbox driver and tests Mario Six
2018-05-23 16:33   ` Simon Glass
2018-05-25 11:19     ` Mario Six
2018-05-23 12:09 ` [U-Boot] [PATCH v2 4/5] lib: Import hexdump.c from Linux Mario Six
2018-05-23 16:33   ` Simon Glass
2018-05-25 11:22     ` Mario Six
2018-06-04 16:05   ` Alexey Brodkin
2018-06-13  8:24     ` Mario Six
2018-06-13 16:04       ` Alexey Brodkin
2018-05-23 12:09 ` [U-Boot] [PATCH v2 5/5] cmd: Add osd commands Mario Six
2018-05-23 16:33   ` Simon Glass
2018-05-25 11:23     ` Mario Six
2018-05-23 16:33 ` [U-Boot] [PATCH v2 1/5] drivers: Add OSD uclass Simon Glass
2018-05-25 11:25   ` Mario Six

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.