[PATCH v3 0/3] drm: describe display bus format

[PATCH v3 0/3] drm: describe display bus format

[Boris Brezillon]

Hello,

This series makes use of the MEDIA_BUS_FMT definition to describe how
the data are transmitted to the display.

This will allow drivers to configure their output display bus according
to the display capabilities.
For example some display controllers support DPI (or raw RGB) connectors
and need to specify which format will be transmitted on the DPI bus
(RGB444, RGB565, RGB888, ...).

This series also adds a field to the panel_desc struct so that one
can specify which format is natevely supported by a panel.

Regards,

Boris

Changes since v2:
 - use the MEDIA_BUS_FMT macros

Changes since v1:
 - rename nformats into num_formats
 - declare num_formats as an unsigned int

Boris Brezillon (3):
  drm: add bus_formats and nbus_formats fields to drm_display_info
  drm: panel: simple-panel: add support for bus_format retrieval
  drm: panel: simple-panel: add bus format information for foxlink panel

 drivers/gpu/drm/drm_crtc.c           | 30 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/panel/panel-simple.c |  6 ++++++
 include/drm/drm_crtc.h               |  7 +++++++
 3 files changed, 43 insertions(+)

-- 
1.9.1

[PATCH v3 1/3] drm: add bus_formats and nbus_formats fields to drm_display_info

[Boris Brezillon]

Add bus_formats and nbus_formats fields and
drm_display_info_set_bus_formats helper function to specify the bus
formats supported by a given display.

This information can be used by display controller drivers to configure
the output interface appropriately (i.e. RGB565, RGB666 or RGB888 on raw
RGB or LVDS busses).

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/gpu/drm/drm_crtc.c | 30 ++++++++++++++++++++++++++++++
 include/drm/drm_crtc.h     |  7 +++++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e79c8d3..17e3acf 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -763,6 +763,36 @@ static void drm_mode_remove(struct drm_connector *connector,
 	drm_mode_destroy(connector->dev, mode);
 }
 
+/*
+ * drm_display_info_set_bus_formats - set the supported bus formats
+ * @info: display info to store bus formats in
+ * @fmts: array containing the supported bus formats
+ * @nfmts: the number of entries in the fmts array
+ *
+ * Store the suppported bus formats in display info structure.
+ */
+int drm_display_info_set_bus_formats(struct drm_display_info *info, const u32 *fmts,
+				     unsigned int num_fmts)
+{
+	u32 *formats = NULL;
+
+	if (!fmts && num_fmts)
+		return -EINVAL;
+
+	if (fmts && num_fmts) {
+		formats = kmemdup(fmts, sizeof(*fmts) * num_fmts, GFP_KERNEL);
+		if (!formats)
+			return -ENOMEM;
+	}
+
+	kfree(info->bus_formats);
+	info->bus_formats = formats;
+	info->num_bus_formats = num_fmts;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_display_info_set_bus_formats);
+
 /**
  * drm_connector_get_cmdline_mode - reads the user's cmdline mode
  * @connector: connector to quwery
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index c40070a..2e0a3e8 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -31,6 +31,7 @@
 #include <linux/idr.h>
 #include <linux/fb.h>
 #include <linux/hdmi.h>
+#include <linux/media-bus-format.h>
 #include <uapi/drm/drm_mode.h>
 #include <uapi/drm/drm_fourcc.h>
 #include <drm/drm_modeset_lock.h>
@@ -130,6 +131,9 @@ struct drm_display_info {
 	enum subpixel_order subpixel_order;
 	u32 color_formats;
 
+	const u32 *bus_formats;
+	int num_bus_formats;
+
 	/* Mask of supported hdmi deep color modes */
 	u8 edid_hdmi_dc_modes;
 
@@ -982,6 +986,9 @@ extern int drm_mode_connector_set_path_property(struct drm_connector *connector,
 extern int drm_mode_connector_update_edid_property(struct drm_connector *connector,
 						struct edid *edid);
 
+extern int drm_display_info_set_bus_formats(struct drm_display_info *info,
+					    const u32 *fmts, unsigned int nfmts);
+
 static inline bool drm_property_type_is(struct drm_property *property,
 		uint32_t type)
 {
-- 
1.9.1

Re: [PATCH v3 1/3] drm: add bus_formats and nbus_formats fields to drm_display_info

[Laurent Pinchart]

Hi Boris,

Thank you for the patch. I just have two small comments.

On Tuesday 18 November 2014 14:46:18 Boris Brezillon wrote:
> Add bus_formats and nbus_formats fields and
> drm_display_info_set_bus_formats helper function to specify the bus
> formats supported by a given display.
> 
> This information can be used by display controller drivers to configure
> the output interface appropriately (i.e. RGB565, RGB666 or RGB888 on raw
> RGB or LVDS busses).
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>  drivers/gpu/drm/drm_crtc.c | 30 ++++++++++++++++++++++++++++++
>  include/drm/drm_crtc.h     |  7 +++++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index e79c8d3..17e3acf 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -763,6 +763,36 @@ static void drm_mode_remove(struct drm_connector
> *connector, drm_mode_destroy(connector->dev, mode);
>  }
> 
> +/*
> + * drm_display_info_set_bus_formats - set the supported bus formats
> + * @info: display info to store bus formats in
> + * @fmts: array containing the supported bus formats
> + * @nfmts: the number of entries in the fmts array
> + *
> + * Store the suppported bus formats in display info structure.

Could you document that the formats are specified as MEDIA_BUS_FMT_* values ?

> + */
> +int drm_display_info_set_bus_formats(struct drm_display_info *info, const
> u32 *fmts, +				     unsigned int num_fmts)
> +{
> +	u32 *formats = NULL;
> +
> +	if (!fmts && num_fmts)
> +		return -EINVAL;
> +
> +	if (fmts && num_fmts) {
> +		formats = kmemdup(fmts, sizeof(*fmts) * num_fmts, GFP_KERNEL);
> +		if (!formats)
> +			return -ENOMEM;
> +	}
> +
> +	kfree(info->bus_formats);
> +	info->bus_formats = formats;
> +	info->num_bus_formats = num_fmts;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_display_info_set_bus_formats);
> +
>  /**
>   * drm_connector_get_cmdline_mode - reads the user's cmdline mode
>   * @connector: connector to quwery
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index c40070a..2e0a3e8 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -31,6 +31,7 @@
>  #include <linux/idr.h>
>  #include <linux/fb.h>
>  #include <linux/hdmi.h>
> +#include <linux/media-bus-format.h>
>  #include <uapi/drm/drm_mode.h>
>  #include <uapi/drm/drm_fourcc.h>
>  #include <drm/drm_modeset_lock.h>
> @@ -130,6 +131,9 @@ struct drm_display_info {
>  	enum subpixel_order subpixel_order;
>  	u32 color_formats;
> 
> +	const u32 *bus_formats;
> +	int num_bus_formats;

As the number of formats is never negative, I would make it an unsigned int.

> +
>  	/* Mask of supported hdmi deep color modes */
>  	u8 edid_hdmi_dc_modes;
> 
> @@ -982,6 +986,9 @@ extern int drm_mode_connector_set_path_property(struct
> drm_connector *connector, extern int
> drm_mode_connector_update_edid_property(struct drm_connector *connector,
> struct edid *edid);
> 
> +extern int drm_display_info_set_bus_formats(struct drm_display_info *info,
> +					    const u32 *fmts, unsigned int nfmts);
> +
>  static inline bool drm_property_type_is(struct drm_property *property,
>  		uint32_t type)
>  {

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 1/3] drm: add bus_formats and nbus_formats fields to drm_display_info

[Boris Brezillon]

Hi Laurent,

On Sat, 29 Nov 2014 00:13:47 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> Hi Boris,
> 
> Thank you for the patch. I just have two small comments.
> 
> On Tuesday 18 November 2014 14:46:18 Boris Brezillon wrote:
> > Add bus_formats and nbus_formats fields and
> > drm_display_info_set_bus_formats helper function to specify the bus
> > formats supported by a given display.
> > 
> > This information can be used by display controller drivers to configure
> > the output interface appropriately (i.e. RGB565, RGB666 or RGB888 on raw
> > RGB or LVDS busses).
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > ---
> >  drivers/gpu/drm/drm_crtc.c | 30 ++++++++++++++++++++++++++++++
> >  include/drm/drm_crtc.h     |  7 +++++++
> >  2 files changed, 37 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index e79c8d3..17e3acf 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -763,6 +763,36 @@ static void drm_mode_remove(struct drm_connector
> > *connector, drm_mode_destroy(connector->dev, mode);
> >  }
> > 
> > +/*
> > + * drm_display_info_set_bus_formats - set the supported bus formats
> > + * @info: display info to store bus formats in
> > + * @fmts: array containing the supported bus formats
> > + * @nfmts: the number of entries in the fmts array
> > + *
> > + * Store the suppported bus formats in display info structure.
> 
> Could you document that the formats are specified as MEDIA_BUS_FMT_* values ?

Sure, I'll clearly state that.

> 
> > + */
> > +int drm_display_info_set_bus_formats(struct drm_display_info *info, const
> > u32 *fmts, +				     unsigned int num_fmts)
> > +{
> > +	u32 *formats = NULL;
> > +
> > +	if (!fmts && num_fmts)
> > +		return -EINVAL;
> > +
> > +	if (fmts && num_fmts) {
> > +		formats = kmemdup(fmts, sizeof(*fmts) * num_fmts, GFP_KERNEL);
> > +		if (!formats)
> > +			return -ENOMEM;
> > +	}
> > +
> > +	kfree(info->bus_formats);
> > +	info->bus_formats = formats;
> > +	info->num_bus_formats = num_fmts;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(drm_display_info_set_bus_formats);
> > +
> >  /**
> >   * drm_connector_get_cmdline_mode - reads the user's cmdline mode
> >   * @connector: connector to quwery
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index c40070a..2e0a3e8 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -31,6 +31,7 @@
> >  #include <linux/idr.h>
> >  #include <linux/fb.h>
> >  #include <linux/hdmi.h>
> > +#include <linux/media-bus-format.h>
> >  #include <uapi/drm/drm_mode.h>
> >  #include <uapi/drm/drm_fourcc.h>
> >  #include <drm/drm_modeset_lock.h>
> > @@ -130,6 +131,9 @@ struct drm_display_info {
> >  	enum subpixel_order subpixel_order;
> >  	u32 color_formats;
> > 
> > +	const u32 *bus_formats;
> > +	int num_bus_formats;
> 
> As the number of formats is never negative, I would make it an unsigned int.

Okay, I'll make it an unsigned int.

Regards,

Boris


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v3 2/3] drm: panel: simple-panel: add support for bus_format retrieval

[Boris Brezillon]

Provide a way to specify panel requirement in terms of supported media bus
format (particularly useful for panels connected to an RGB or LVDS bus).

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/gpu/drm/panel/panel-simple.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 23de22f..66838a5 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -61,6 +61,8 @@ struct panel_desc {
 		unsigned int disable;
 		unsigned int unprepare;
 	} delay;
+
+	u32 bus_format;
 };
 
 struct panel_simple {
@@ -111,6 +113,9 @@ static int panel_simple_get_fixed_modes(struct panel_simple *panel)
 	connector->display_info.bpc = panel->desc->bpc;
 	connector->display_info.width_mm = panel->desc->size.width;
 	connector->display_info.height_mm = panel->desc->size.height;
+	if (panel->desc->bus_format)
+		drm_display_info_set_bus_formats(&connector->display_info,
+						 &panel->desc->bus_format, 1);
 
 	return num;
 }
-- 
1.9.1

[PATCH v3 3/3] drm: panel: simple-panel: add bus format information for foxlink panel

[Boris Brezillon]

Foxlink's fl500wvr00-a0t supports RGB888 format.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/gpu/drm/panel/panel-simple.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 66838a5..695f406 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -545,6 +545,7 @@ static const struct panel_desc foxlink_fl500wvr00_a0t = {
 		.width = 108,
 		.height = 65,
 	},
+	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
 };
 
 static const struct drm_display_mode innolux_n116bge_mode = {
-- 
1.9.1

Re: [PATCH v3 0/3] drm: describe display bus format

[Boris Brezillon]

Hi,

On Tue, 18 Nov 2014 14:46:17 +0100
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> Hello,
> 
> This series makes use of the MEDIA_BUS_FMT definition to describe how
> the data are transmitted to the display.
> 
> This will allow drivers to configure their output display bus according
> to the display capabilities.
> For example some display controllers support DPI (or raw RGB) connectors
> and need to specify which format will be transmitted on the DPI bus
> (RGB444, RGB565, RGB888, ...).
> 
> This series also adds a field to the panel_desc struct so that one
> can specify which format is natevely supported by a panel.

Thierry, Laurent, Dave, can you take a look at this patch series: this
is the last missing dependency to get the atmel-hlcdc DRM driver
mainlined, and I was expecting to get this driver in 3.19...

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v3 0/3] drm: describe display bus format

[Laurent Pinchart]

Hi Boris,

On Thursday 27 November 2014 14:37:50 Boris Brezillon wrote:
> On Tue, 18 Nov 2014 14:46:17 +0100 Boris Brezillon wrote:
> > Hello,
> > 
> > This series makes use of the MEDIA_BUS_FMT definition to describe how
> > the data are transmitted to the display.
> > 
> > This will allow drivers to configure their output display bus according
> > to the display capabilities.
> > For example some display controllers support DPI (or raw RGB) connectors
> > and need to specify which format will be transmitted on the DPI bus
> > (RGB444, RGB565, RGB888, ...).
> > 
> > This series also adds a field to the panel_desc struct so that one
> > can specify which format is natevely supported by a panel.
> 
> Thierry, Laurent, Dave, can you take a look at this patch series: this
> is the last missing dependency to get the atmel-hlcdc DRM driver
> mainlined, and I was expecting to get this driver in 3.19...

I've reviewed the series, it looks globally fine to me. I just had two small 
comments on patch 1/3.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 0/3] drm: describe display bus format

[Boris Brezillon]

Hi Laurent,

On Sat, 29 Nov 2014 00:29:10 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> Hi Boris,
> 
> On Thursday 27 November 2014 14:37:50 Boris Brezillon wrote:
> > On Tue, 18 Nov 2014 14:46:17 +0100 Boris Brezillon wrote:
> > > Hello,
> > > 
> > > This series makes use of the MEDIA_BUS_FMT definition to describe how
> > > the data are transmitted to the display.
> > > 
> > > This will allow drivers to configure their output display bus according
> > > to the display capabilities.
> > > For example some display controllers support DPI (or raw RGB) connectors
> > > and need to specify which format will be transmitted on the DPI bus
> > > (RGB444, RGB565, RGB888, ...).
> > > 
> > > This series also adds a field to the panel_desc struct so that one
> > > can specify which format is natevely supported by a panel.
> > 
> > Thierry, Laurent, Dave, can you take a look at this patch series: this
> > is the last missing dependency to get the atmel-hlcdc DRM driver
> > mainlined, and I was expecting to get this driver in 3.19...
> 
> I've reviewed the series, it looks globally fine to me. I just had two small 
> comments on patch 1/3.
> 

Thanks for the review, I'll address your comments in the next version.

Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com