diff mbox series

[v1,2/9] drm: add drm specific media-bus-format header file

Message ID 20220206154405.1243333-3-sam@ravnborg.org (mailing list archive)
State New, archived
Headers show
Series drm/bridge: ps8640 and ti-sn65dsi86 updates | expand

Commit Message

Sam Ravnborg Feb. 6, 2022, 3:43 p.m. UTC 
For now the header file includes a single method to retreive the bpc
from the bus format.
The supported MEDIA_BUS_* codes are the ones used for the current panels
in DRM. The list can be extended as there are a need for more formats.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
---
 include/drm/media-bus-format.h | 53 ++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 include/drm/media-bus-format.h

Comments

Doug Anderson Feb. 7, 2022, 10:21 p.m. UTC | #1 
Hi,

On Sun, Feb 6, 2022 at 7:44 AM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> For now the header file includes a single method to retreive the bpc
> from the bus format.
> The supported MEDIA_BUS_* codes are the ones used for the current panels
> in DRM. The list can be extended as there are a need for more formats.
>
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> ---
>  include/drm/media-bus-format.h | 53 ++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>  create mode 100644 include/drm/media-bus-format.h
>
> diff --git a/include/drm/media-bus-format.h b/include/drm/media-bus-format.h
> new file mode 100644
> index 000000000000..d4d18f19a70f
> --- /dev/null
> +++ b/include/drm/media-bus-format.h
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2022 Sam Ravnborg
> + */
> +
> +#ifndef __LINUX_DRM_MEDIA_BUS_FORMAT
> +#define __LINUX_DRM_MEDIA_BUS_FORMAT
> +
> +#include <linux/bug.h>
> +#include <linux/media-bus-format.h>
> +#include <linux/types.h>
> +
> +/**
> + * media_bus_format_to_bpc - The bits per color channel for the bus_format
> + *
> + * Based on the supplied bus_format return the maximum number of bits
> + * per color channel.
> + *
> + * RETURNS
> + * The number of bits per color channel, or -EINVAL if the bus_format
> + * is unknown.

kernel-doc doesn't like your syntax quite right. Try running:

./scripts/kernel-doc -rst -v include/drm/media-bus-format.h

It will tell you that you're missing a description of the parameter
and the way you're describing the return value isn't in a way that it
can parse...

> + */
> +static inline int media_bus_format_to_bpc(u32 bus_format)
> +{
> +       switch (bus_format) {
> +       /* DPI */
> +       case MEDIA_BUS_FMT_RGB565_1X16:
> +       case MEDIA_BUS_FMT_RGB666_1X18:
> +               return 6;
> +
> +       /* DPI */
> +       case MEDIA_BUS_FMT_RGB888_1X24:
> +       case MEDIA_BUS_FMT_RGB888_3X8:
> +       case MEDIA_BUS_FMT_RGB888_3X8_DELTA:
> +       case MEDIA_BUS_FMT_Y8_1X8:
> +               return 8;
> +
> +       /* LVDS */
> +       case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
> +               return 6;
> +
> +       /* LVDS */

When applying your patches, I got a warning that both of your "/* LVDS
*/" comments have spaces before the tab.

I'm also not sure what the "sections" are supposed to mean. Are you
saying that the bus formats are only for the given transport type?
...so we can't use MEDIA_BUS_FMT_RGB666_1X18 for eDP?

-Doug
Laurent Pinchart Feb. 8, 2022, 12:40 a.m. UTC | #2 
Hi Sam,

Thank you for the patch.

On Sun, Feb 06, 2022 at 04:43:58PM +0100, Sam Ravnborg wrote:
> For now the header file includes a single method to retreive the bpc

s/retreive/retrieve/

> from the bus format.
> The supported MEDIA_BUS_* codes are the ones used for the current panels
> in DRM. The list can be extended as there are a need for more formats.
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> ---
>  include/drm/media-bus-format.h | 53 ++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>  create mode 100644 include/drm/media-bus-format.h
> 
> diff --git a/include/drm/media-bus-format.h b/include/drm/media-bus-format.h
> new file mode 100644
> index 000000000000..d4d18f19a70f
> --- /dev/null
> +++ b/include/drm/media-bus-format.h
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2022 Sam Ravnborg
> + */
> +
> +#ifndef __LINUX_DRM_MEDIA_BUS_FORMAT
> +#define __LINUX_DRM_MEDIA_BUS_FORMAT
> +
> +#include <linux/bug.h>
> +#include <linux/media-bus-format.h>
> +#include <linux/types.h>
> +
> +/**
> + * media_bus_format_to_bpc - The bits per color channel for the bus_format
> + *
> + * Based on the supplied bus_format return the maximum number of bits
> + * per color channel.
> + *
> + * RETURNS
> + * The number of bits per color channel, or -EINVAL if the bus_format
> + * is unknown.
> + */
> +static inline int media_bus_format_to_bpc(u32 bus_format)
> +{
> +	switch (bus_format) {
> +	/* DPI */
> +	case MEDIA_BUS_FMT_RGB565_1X16:
> +	case MEDIA_BUS_FMT_RGB666_1X18:
> +		return 6;
> +
> +	/* DPI */
> +	case MEDIA_BUS_FMT_RGB888_1X24:
> +	case MEDIA_BUS_FMT_RGB888_3X8:
> +	case MEDIA_BUS_FMT_RGB888_3X8_DELTA:
> +	case MEDIA_BUS_FMT_Y8_1X8:
> +		return 8;
> +
> +     	/* LVDS */

Wrong indentation.

> +	case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
> +		return 6;
> +
> +     	/* LVDS */
> +	case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
> +	case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
> +		return 8;
> +
> +	default:
> +		WARN(1, "Unknown MEDIA_BUS format %d\n", bus_format);
> +		return -EINVAL;
> +	}
> +}

This seems a bit big for an inline function.

If we add more helper functions, it will result in lots of switch...case
statements. Could we use the same approach as in drm_fourcc.h with a
format info structure ?

> +
> +#endif
diff mbox series

Patch

diff --git a/include/drm/media-bus-format.h b/include/drm/media-bus-format.h
new file mode 100644
index 000000000000..d4d18f19a70f
--- /dev/null
+++ b/include/drm/media-bus-format.h
@@ -0,0 +1,53 @@ 
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2022 Sam Ravnborg
+ */
+
+#ifndef __LINUX_DRM_MEDIA_BUS_FORMAT
+#define __LINUX_DRM_MEDIA_BUS_FORMAT
+
+#include <linux/bug.h>
+#include <linux/media-bus-format.h>
+#include <linux/types.h>
+
+/**
+ * media_bus_format_to_bpc - The bits per color channel for the bus_format
+ *
+ * Based on the supplied bus_format return the maximum number of bits
+ * per color channel.
+ *
+ * RETURNS
+ * The number of bits per color channel, or -EINVAL if the bus_format
+ * is unknown.
+ */
+static inline int media_bus_format_to_bpc(u32 bus_format)
+{
+	switch (bus_format) {
+	/* DPI */
+	case MEDIA_BUS_FMT_RGB565_1X16:
+	case MEDIA_BUS_FMT_RGB666_1X18:
+		return 6;
+
+	/* DPI */
+	case MEDIA_BUS_FMT_RGB888_1X24:
+	case MEDIA_BUS_FMT_RGB888_3X8:
+	case MEDIA_BUS_FMT_RGB888_3X8_DELTA:
+	case MEDIA_BUS_FMT_Y8_1X8:
+		return 8;
+
+     	/* LVDS */
+	case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
+		return 6;
+
+     	/* LVDS */
+	case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
+	case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
+		return 8;
+
+	default:
+		WARN(1, "Unknown MEDIA_BUS format %d\n", bus_format);
+		return -EINVAL;
+	}
+}
+
+#endif