[U-Boot,1/3] drivers: Add OSD uclass

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>
---
 drivers/video/Kconfig            |   8 +++
 drivers/video/Makefile           |   2 +
 drivers/video/video_osd-uclass.c |  47 +++++++++++++
 include/dm/uclass-id.h           |   1 +
 include/video_osd.h              | 145 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 203 insertions(+)
 create mode 100644 drivers/video/video_osd-uclass.c
 create mode 100644 include/video_osd.h

Hi Mario,

On 28 March 2018 at 20:39, 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>
> ---
>  drivers/video/Kconfig            |   8 +++
>  drivers/video/Makefile           |   2 +
>  drivers/video/video_osd-uclass.c |  47 +++++++++++++
>  include/dm/uclass-id.h           |   1 +
>  include/video_osd.h              | 145 +++++++++++++++++++++++++++++++++++++++
>  5 files changed, 203 insertions(+)
>  create mode 100644 drivers/video/video_osd-uclass.c
>  create mode 100644 include/video_osd.h

Please add a sandbox driver and simple test for this. All uclasses
should have this.

>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 2fc0defcd0..da60bbe692 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -660,4 +660,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 dfafe08fc5..209d5e3a75 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -58,5 +58,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 0000000000..b6dd7e59b1
> --- /dev/null
> +++ b/drivers/video/video_osd-uclass.c
> @@ -0,0 +1,47 @@
> +/*
> + * (C) Copyright 2017
> + * Mario Six,  Guntermann & Drunck GmbH, mario.six@gdsys.cc
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <video_osd.h>
> +
> +int video_osd_get_data(struct udevice *dev, void *data)
> +{
> +       struct video_osd_ops *ops = video_osd_get_ops(dev);
> +
> +       return ops->get_data(dev, data);
> +}
> +
> +int video_osd_set_mem(struct udevice *dev, uint x, uint y, u8 *buf,
> +                     size_t buflen, uint count)
> +{
> +       struct video_osd_ops *ops = video_osd_get_ops(dev);
> +
> +       return ops->set_mem(dev, x, y, buf, buflen, count);
> +}
> +
> +int video_osd_set_size(struct udevice *dev, uint x, uint y)
> +{
> +       struct video_osd_ops *ops = video_osd_get_ops(dev);
> +
> +       return ops->set_size(dev, x, y);
> +}
> +
> +int video_osd_print(struct udevice *dev, uint x, uint y, ulong color,
> +                   char *text)
> +{
> +       struct video_osd_ops *ops = video_osd_get_ops(dev);
> +
> +       return ops->print(dev, x, y, 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 07fabc3ce6..685f22550a 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 displays */

please use singular

>         UCLASS_WDT,             /* Watchdot Timer driver */
>
>         UCLASS_COUNT,
> diff --git a/include/video_osd.h b/include/video_osd.h
> new file mode 100644
> index 0000000000..1ab5ff9b21
> --- /dev/null
> +++ b/include/video_osd.h
> @@ -0,0 +1,145 @@
> +/*
> + * (C) Copyright 2017
> + * Mario Six,  Guntermann & Drunck GmbH, mario.six@gdsys.cc
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#ifndef _VIDEO_OSD_H_
> +#define _VIDEO_OSD_H_
> +
> +/**
> + * struct video_osd_ops - driver operations for OSD uclass
> + *

Please add a general description of what this uclass is and how it works

> + * 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_data() - 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.

How big is the data? What does it contain? Should pass in the size of
the buffer I think.

> +        *
> +        * @dev:        OSD instance to query.
> +        * @data:       Pointer to a buffer that takes the information read
> +        *              from the OSD instance.
> +        * @return 0 if OK, -ve on error.
> +        */
> +       int (*get_data)(struct udevice *dev, void *data);
> +
> +       /**
> +        * set_mem() - Write pixel 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.
> +        *
> +        * @dev:        OSD instance to write to.
> +        * @x:          Horizontal character coordinate to write to.
> +        * @y:          Vertical character coordinate to write to.
> +        * @buf:        Array containing data to write to the specified address
> +        *              in the OSD memory.
> +        * @buflen:     Length of the data in the passed buffer (in byte).
> +        * @count:      Write count many repetitions of the given pixel data

Can you expand this a bit?

> +        * @return 0 if OK, -ve on error.
> +        */
> +       int (*set_mem)(struct udevice *dev, uint x, uint y, 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.
> +        * @x:          The number of characters in the window's columns
> +        * @y:          The number of characters in the window's rows

Is this character rows/columns or pixels? Should update docs.

> +        * @return 0 if OK, -ve on error.
> +        */
> +       int (*set_size)(struct udevice *dev, uint x, uint y);
> +
> +       /**
> +        * print() - Print a string in a given color to specified coordinates
> +        *           on the OSD
> +        *
> +        * @dev:        OSD instance to write to.
> +        * @x:          The x-coordinate of the position the string should be
> +        *              written to
> +        * @y:          The y-coordinate of the position the string should be
> +        *              written to
> +        * @color:      The color in which the specified string should be
> +        *              printed

What is the value for colour? Please expand docs to explain this.


> +        * @text:       The string data that should be printed on the OSD
> +        * @return 0 if OK, -ve on error.
> +        */
> +       int (*print)(struct udevice *dev, uint x, uint y, ulong color,
> +                    char *text);
> +};
> +
> +#define video_osd_get_ops(dev) ((struct video_osd_ops *)(dev)->driver->ops)
> +
> +/**
> + * video_osd_get_data() - 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.
> + * @data:      Pointer to a buffer that takes the information read from the
> + *             OSD instance.
> + * @return 0 if OK, -ve on error.
> + */
> +int video_osd_get_data(struct udevice *dev, void *data);
> +
> +/**
> + * video_osd_set_mem() - Write pixel 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.
> + *
> + * @dev:       OSD instance to write to.
> + * @x:         Horizontal character coordinate to write to.
> + * @y:         Vertical character coordinate to write to.
> + * @buf:       Array containing data to write to the specified address in the
> + *             OSD memory.
> + * @buflen:    Length of the data in the passed buffer (in byte).
> + * @count:     Write count many repetitions of the given pixel data
> + * @return 0 if OK, -ve on error.
> + */
> +int video_osd_set_mem(struct udevice *dev, uint x, uint y, 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.
> + * @x:         The number of characters in the window's columns
> + * @y:         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 x, uint y);
> +
> +/**
> + * video_osd_print() - Print a string in a given color to specified coordinates
> + *                    on the OSD
> + *
> + * @dev:       OSD instance to write to.
> + * @x:         The x-coordinate of the position the string should be written
> + *             to
> + * @y:         The y-coordinate of the position the string should be written
> + *             to
> + * @color:     The color in which the specified string should be printed
> + * @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 x, uint y, ulong color,
> +                   char *text);
> +
> +#endif /* !_VIDEO_OSD_H_ */
> --
> 2.16.1
>

Regards,
Simon

Hi Simon,

On Fri, Mar 30, 2018 at 12:42 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Mario,
>
> On 28 March 2018 at 20:39, 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>
>> ---
>>  drivers/video/Kconfig            |   8 +++
>>  drivers/video/Makefile           |   2 +
>>  drivers/video/video_osd-uclass.c |  47 +++++++++++++
>>  include/dm/uclass-id.h           |   1 +
>>  include/video_osd.h              | 145 +++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 203 insertions(+)
>>  create mode 100644 drivers/video/video_osd-uclass.c
>>  create mode 100644 include/video_osd.h
>
> Please add a sandbox driver and simple test for this. All uclasses
> should have this.
>
>>
>> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
>> index 2fc0defcd0..da60bbe692 100644
>> --- a/drivers/video/Kconfig
>> +++ b/drivers/video/Kconfig
>> @@ -660,4 +660,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 dfafe08fc5..209d5e3a75 100644
>> --- a/drivers/video/Makefile
>> +++ b/drivers/video/Makefile
>> @@ -58,5 +58,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 0000000000..b6dd7e59b1
>> --- /dev/null
>> +++ b/drivers/video/video_osd-uclass.c
>> @@ -0,0 +1,47 @@
>> +/*
>> + * (C) Copyright 2017
>> + * Mario Six,  Guntermann & Drunck GmbH, mario.six@gdsys.cc
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <video_osd.h>
>> +
>> +int video_osd_get_data(struct udevice *dev, void *data)
>> +{
>> +       struct video_osd_ops *ops = video_osd_get_ops(dev);
>> +
>> +       return ops->get_data(dev, data);
>> +}
>> +
>> +int video_osd_set_mem(struct udevice *dev, uint x, uint y, u8 *buf,
>> +                     size_t buflen, uint count)
>> +{
>> +       struct video_osd_ops *ops = video_osd_get_ops(dev);
>> +
>> +       return ops->set_mem(dev, x, y, buf, buflen, count);
>> +}
>> +
>> +int video_osd_set_size(struct udevice *dev, uint x, uint y)
>> +{
>> +       struct video_osd_ops *ops = video_osd_get_ops(dev);
>> +
>> +       return ops->set_size(dev, x, y);
>> +}
>> +
>> +int video_osd_print(struct udevice *dev, uint x, uint y, ulong color,
>> +                   char *text)
>> +{
>> +       struct video_osd_ops *ops = video_osd_get_ops(dev);
>> +
>> +       return ops->print(dev, x, y, 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 07fabc3ce6..685f22550a 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 displays */
>
> please use singular
>
>>         UCLASS_WDT,             /* Watchdot Timer driver */
>>
>>         UCLASS_COUNT,
>> diff --git a/include/video_osd.h b/include/video_osd.h
>> new file mode 100644
>> index 0000000000..1ab5ff9b21
>> --- /dev/null
>> +++ b/include/video_osd.h
>> @@ -0,0 +1,145 @@
>> +/*
>> + * (C) Copyright 2017
>> + * Mario Six,  Guntermann & Drunck GmbH, mario.six@gdsys.cc
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#ifndef _VIDEO_OSD_H_
>> +#define _VIDEO_OSD_H_
>> +
>> +/**
>> + * struct video_osd_ops - driver operations for OSD uclass
>> + *
>
> Please add a general description of what this uclass is and how it works
>
>> + * 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_data() - 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.
>
> How big is the data? What does it contain? Should pass in the size of
> the buffer I think.
>
>> +        *
>> +        * @dev:        OSD instance to query.
>> +        * @data:       Pointer to a buffer that takes the information read
>> +        *              from the OSD instance.
>> +        * @return 0 if OK, -ve on error.
>> +        */
>> +       int (*get_data)(struct udevice *dev, void *data);
>> +
>> +       /**
>> +        * set_mem() - Write pixel 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.
>> +        *
>> +        * @dev:        OSD instance to write to.
>> +        * @x:          Horizontal character coordinate to write to.
>> +        * @y:          Vertical character coordinate to write to.
>> +        * @buf:        Array containing data to write to the specified address
>> +        *              in the OSD memory.
>> +        * @buflen:     Length of the data in the passed buffer (in byte).
>> +        * @count:      Write count many repetitions of the given pixel data
>
> Can you expand this a bit?
>
>> +        * @return 0 if OK, -ve on error.
>> +        */
>> +       int (*set_mem)(struct udevice *dev, uint x, uint y, 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.
>> +        * @x:          The number of characters in the window's columns
>> +        * @y:          The number of characters in the window's rows
>
> Is this character rows/columns or pixels? Should update docs.
>
>> +        * @return 0 if OK, -ve on error.
>> +        */
>> +       int (*set_size)(struct udevice *dev, uint x, uint y);
>> +
>> +       /**
>> +        * print() - Print a string in a given color to specified coordinates
>> +        *           on the OSD
>> +        *
>> +        * @dev:        OSD instance to write to.
>> +        * @x:          The x-coordinate of the position the string should be
>> +        *              written to
>> +        * @y:          The y-coordinate of the position the string should be
>> +        *              written to
>> +        * @color:      The color in which the specified string should be
>> +        *              printed
>
> What is the value for colour? Please expand docs to explain this.
>
>
>> +        * @text:       The string data that should be printed on the OSD
>> +        * @return 0 if OK, -ve on error.
>> +        */
>> +       int (*print)(struct udevice *dev, uint x, uint y, ulong color,
>> +                    char *text);
>> +};
>> +
>> +#define video_osd_get_ops(dev) ((struct video_osd_ops *)(dev)->driver->ops)
>> +
>> +/**
>> + * video_osd_get_data() - 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.
>> + * @data:      Pointer to a buffer that takes the information read from the
>> + *             OSD instance.
>> + * @return 0 if OK, -ve on error.
>> + */
>> +int video_osd_get_data(struct udevice *dev, void *data);
>> +
>> +/**
>> + * video_osd_set_mem() - Write pixel 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.
>> + *
>> + * @dev:       OSD instance to write to.
>> + * @x:         Horizontal character coordinate to write to.
>> + * @y:         Vertical character coordinate to write to.
>> + * @buf:       Array containing data to write to the specified address in the
>> + *             OSD memory.
>> + * @buflen:    Length of the data in the passed buffer (in byte).
>> + * @count:     Write count many repetitions of the given pixel data
>> + * @return 0 if OK, -ve on error.
>> + */
>> +int video_osd_set_mem(struct udevice *dev, uint x, uint y, 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.
>> + * @x:         The number of characters in the window's columns
>> + * @y:         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 x, uint y);
>> +
>> +/**
>> + * video_osd_print() - Print a string in a given color to specified coordinates
>> + *                    on the OSD
>> + *
>> + * @dev:       OSD instance to write to.
>> + * @x:         The x-coordinate of the position the string should be written
>> + *             to
>> + * @y:         The y-coordinate of the position the string should be written
>> + *             to
>> + * @color:     The color in which the specified string should be printed
>> + * @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 x, uint y, ulong color,
>> +                   char *text);
>> +
>> +#endif /* !_VIDEO_OSD_H_ */
>> --
>> 2.16.1
>>
>
> Regards,
> Simon


I'll address all problems in v2. Thanks for reviewing!

Best regards,

Mario