Hi Simon,
On Wed, Jul 19, 2017 at 11:06 AM, Simon Glass sjg@chromium.org wrote:
Hi Mario,
On 14 July 2017 at 05:55, Mario Six mario.six@gdsys.cc wrote:
Add command to query information from and write text to IHS OSDs.
Signed-off-by: Mario Six mario.six@gdsys.cc
cmd/Kconfig | 6 +++ cmd/Makefile | 1 + cmd/ihs_osd.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 174 insertions(+) create mode 100644 cmd/ihs_osd.c
diff --git a/cmd/Kconfig b/cmd/Kconfig index 6425c425d6..b632049022 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -933,6 +933,12 @@ config CMD_DISPLAY displayed on a simple board-specific display. Implement display_putc() to use it.
+config CMD_IHS_OSD
bool "ihs osd"helpEnable the 'osd' command which allows to query information from andwrite text data to a gdsys OSD.config CMD_LED bool "led" default y if LED diff --git a/cmd/Makefile b/cmd/Makefile index 243f9f45d4..c30511982b 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -62,6 +62,7 @@ obj-$(CONFIG_CMD_FS_GENERIC) += fs.o obj-$(CONFIG_CMD_FUSE) += fuse.o obj-$(CONFIG_CMD_GETTIME) += gettime.o obj-$(CONFIG_CMD_GPIO) += gpio.o +obj-$(CONFIG_CMD_IHS_OSD) += ihs_osd.o obj-$(CONFIG_CMD_I2C) += i2c.o obj-$(CONFIG_CMD_IOTRACE) += iotrace.o obj-$(CONFIG_CMD_HASH) += hash.o diff --git a/cmd/ihs_osd.c b/cmd/ihs_osd.c new file mode 100644 index 0000000000..01ef3eee83 --- /dev/null +++ b/cmd/ihs_osd.c @@ -0,0 +1,167 @@ +/*
- (C) Copyright 2017
- Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc
- based on the gdsys osd driver, which is
- (C) Copyright 2010
- Dirk Eibach, Guntermann & Drunck GmbH, eibach@gdsys.de
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <dm.h> +#include <dm/uclass-internal.h> +#include <i2c.h> +#include <ihs_video_out.h> +#include <malloc.h>
+#define MAX_VIDEOMEM_WIDTH 64 +#define MAX_VIDEOMEM_HEIGHT 32 +#define MAX_X_CHARS 53 +#define MAX_Y_CHARS 26
+size_t hexstr_to_u16_array(char *hexstr, u16 *array, size_t arrsize) +{
size_t pos;for (pos = 0; pos < arrsize; ++pos) {char substr[5];memcpy(substr, hexstr, 4);substr[4] = 0;*array = simple_strtoul(substr, NULL, 16);hexstr += 4;array++;if (*hexstr == 0)break;}return pos;+}
+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;u16 buffer[MAX_VIDEOMEM_WIDTH];size_t buflen;if ((argc < 4) || (strlen(argv[3]) % 4)) {cmd_usage(cmdtp);return 1;}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 = hexstr_to_u16_array(hexstr, buffer, MAX_VIDEOMEM_WIDTH);for (uclass_find_first_device(UCLASS_IHS_VIDEO_OUT, &dev);dev;uclass_find_next_device(&dev)) {Why write to all devices? Perhaps you should have the concept of a current device in this file?
We always write to all possible OSDs on our boards. But for a generic OSD uclass this doesn't really make sense, I realize.
This does create a problem with backwards compatibility, though. See below.
uint k;for (k = 0; k < count; ++k)video_out_set_mem(dev, x + k * buflen, y, buffer,buflen);video_out_set_control(dev, 0x0049);}return 0;+}
+static int do_osd_print(cmd_tbl_t *cmdtp, int flag, int argc,
char * const argv[])+{
struct udevice *dev;u16 buffer[MAX_VIDEOMEM_WIDTH];uint x, y, charcount, len;u8 color;uint k;char *text;if (argc < 5) {cmd_usage(cmdtp);return 1;}x = simple_strtoul(argv[1], NULL, 16);y = simple_strtoul(argv[2], NULL, 16);color = simple_strtoul(argv[3], NULL, 16);text = argv[4];charcount = strlen(text);len = min(charcount, (uint)MAX_VIDEOMEM_WIDTH);for (uclass_find_first_device(UCLASS_IHS_VIDEO_OUT, &dev);dev;uclass_find_next_device(&dev)) {int res;for (k = 0; k < len; ++k)buffer[k] = (text[k] << 8) | color;This is specific to your device. If you are making a generic device you should have something like video_osd_set_char(...k, color).
Then other drivers can implement it.
OK, will implement a generic interface in v2 if the backwards compatibility problem can be solved, see below.
res = video_out_set_mem(dev, x, y, buffer, len);if (res < 0)return res;video_out_set_control(dev, 0x0049);}return 0;+}
+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 1;}x = simple_strtoul(argv[1], NULL, 16);y = simple_strtoul(argv[2], NULL, 16);if (!x || (x > 64) || (x > MAX_X_CHARS) ||!y || (y > 32) || (y > MAX_Y_CHARS)) {cmd_usage(cmdtp);return 1;}for (uclass_find_first_device(UCLASS_IHS_VIDEO_OUT, &dev);dev;uclass_find_next_device(&dev))video_out_set_size(dev, ((x - 1) << 8) | (y - 1),32767 * (640 - 12 * x) / 65535,32767 * (480 - 18 * x) / 65535);Again this logic should be in the driver, not the command.
Dito, will change in v2, if the backwards compatibility problem can be solved, see below.
return 0;+}
+U_BOOT_CMD(
osdw, 5, 0, do_osd_write,"write 16-bit hex encoded buffer to osd memory","osd write [pos_x] [pos_y] [buffer] [count] - write 16-bit hex encoded buffer to osd memory\n"+);
+U_BOOT_CMD(
osdp, 5, 0, do_osd_print,"write ASCII buffer to osd memory","osd print [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","osd size [size_x] [size_y] - set OSD XY size in characters\n"+);
2.11.0
I think this should be an 'osd' command with sub-commands.
That's how I first implemented it. Then I realized that these commands have to be backwards compatible to the ones in boards/gdsys/common/osd.c. These demand that data is queried from/written to all OSD instances, and they define three commands instead of one with three subcommands.
How can I keep backwards compatibility intact while still implementing a generic interface for OSDs?
Regards, Simon
Best regards,
Mario