[RFC PATCH 0/3] allow override of bus format in bridges

[RFC PATCH 0/3] allow override of bus format in bridges

[Peter Rosin]

I'm trying to get something to work that I assumed would not
need patches, so I think I might be missing something completely
obvious...

I have an Atmel sama5d31 hooked up to an lvds encoder and then
on to an lvds panel. Which seems like something that has been
done one or two times before...

The problem (I think) is that the bus_format of the SoC and the
panel do not agree. The SoC driver (atmel-hlcdc) can handle the
rgb444, rgb565, rgb666 and rgb888 bus formats. The hardware is
wired for the rgb565 case. The lvds encoder supports rgb888 on
its input side with the LSB wires simply pulled down internally
in the encoder in my case. And the panel is expecting lvds
(vesa-24), which is what the encoder outputs.

The reason I "blame" the bus_format of the drm_connector is that
with the below DT snippet, things do not work *exactly* due to
that. At least, it starts to work if I hack the panel-lvds driver
to report the rgb565 bus_format instead of vesa-24.

	panel: panel {
		compatible = "panel-lvds";

		width-mm = <476>;
		height-mm = <268>;

		data-mapping = "vesa-24";

		panel-timing {
			// 1024x768 @ 60Hz (typical)
			clock-frequency = <52140000 65000000 71100000>;
			hactive = <1024>;
			vactive = <768>;
			hfront-porch = <59 107 107>;
			hback-porch = <59 107 107>;
			hsync-len = <59 106 106>;
			vfront-porch = <8 13 14>;
			vback-porch = <8 13 14>;
			vsync-len = <8 12 14>;
		};

		port {
			panel_input: endpoint {
				remote-endpoint = <&lvds_encoder_output>;
			};
		};
	};

	lvds-encoder {
		compatible = "ti,ds90c187", "lvds-encoder";

		ports {
			#address-cells = <1>;
			#size-cells = <0>;

			port@0 {
				reg = <0>;

				lvds_encoder_input: endpoint {
					remote-endpoint = <&hlcdc_output>;
				};
			};

			port@1 {
				reg = <1>;

				lvds_encoder_output: endpoint {
					remote-endpoint = <&panel_input>;
				};
			};
		};
	};

But, instead of perverting the panel-lvds driver with support
for a totally fake non-lvds bus_format, I came up with an optional
bus_format override in the lvds-encoder driver, which I trigger with
this addition to the lvds-encoder DT node:

		interface-pix-fmt = "rgb565";

There are some issues with the interface-pix-fmt name. I copied it
from the freescale fsl-imx-drm binding, but a) I personally don't
like how that binding uses rgb24 instead of rgb888 and b) I don't
like how the implementation of that binding selects rgb666 when you
say bgr666 in the DT, but for parallel hardware interface the bit
order is not really relevant so I swallowed that. Anyway, it might
be better to use something else than interface-pix-fmt? Perhaps
reuse the "data-mapping" name from the panel-lvds binding? But that
seems somewhat lvds specific? Then there a couple of instances of
"bus-format-override" in some dtsi files, but that name is not
mentioned in any binding, nor in (upstream) code...

And maybe it should be fourcc codes or something instead of these
"rgb565" names?

I threw in the first patch, since that is the actual lvds encoder
I have in this case.

Suggestions welcome.

Cheers,
Peter

Peter Rosin (3):
  dt-bindings: display: bridge: lvds-transmitter: add ti,ds90c185
  drm: bridge: panel: allow override of the bus format
  drm: bridge: lvds-encoder: on request, override the bus format

 .../bindings/display/bridge/lvds-transmitter.txt   | 13 +++++++++++++
 drivers/gpu/drm/bridge/lvds-encoder.c              | 18 ++++++++++++++++++
 drivers/gpu/drm/bridge/panel.c                     | 22 +++++++++++++++++++++-
 include/drm/drm_bridge.h                           |  1 +
 4 files changed, 53 insertions(+), 1 deletion(-)

-- 
2.11.0

[RFC PATCH 1/3] dt-bindings: display: bridge: lvds-transmitter: add ti,ds90c185

[Peter Rosin]

Start list of actual chips compatible with "lvds-encoder".

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../devicetree/bindings/display/bridge/lvds-transmitter.txt          | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
index fd39ad34c383..9d09190d9210 100644
--- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
+++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
@@ -24,6 +24,11 @@ Required properties:
 
 - compatible: Must be "lvds-encoder"
 
+	      Known actual chips (these should still use "lvds-encoder" as a
+	      fallback compatible) include:
+
+	      "ti,ds90c185"
+
 Required nodes:
 
 This device has two video ports. Their connections are modeled using the OF
-- 
2.11.0

Re: [RFC PATCH 1/3] dt-bindings: display: bridge: lvds-transmitter: add ti,ds90c185

[Laurent Pinchart]

Hi Peter,

Thank you for the patch.

On Sunday, 18 March 2018 00:15:23 EET Peter Rosin wrote:
> Start list of actual chips compatible with "lvds-encoder".
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  .../devicetree/bindings/display/bridge/lvds-transmitter.txt       | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> index fd39ad34c383..9d09190d9210 100644
> --- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> @@ -24,6 +24,11 @@ Required properties:
> 
>  - compatible: Must be "lvds-encoder"
> 
> +	      Known actual chips (these should still use "lvds-encoder" as a
> +	      fallback compatible) include:
> +
> +	      "ti,ds90c185"

The wording sounds a bit strange to me. How about

 - compatible: Must be one or more of the following
   - "ti,ds90c185" for the TI DS90C185 FPD-Link Serializer
   - "lvds-encoder" for a generic LVDS encoder device

   When compatible with the generic version nodes must list the
   device-specific version corresponding to the device first
   followed by the generic version.

>  Required nodes:
> 
>  This device has two video ports. Their connections are modeled using the OF

-- 
Regards,

Laurent Pinchart

[RFC PATCH 2/3] drm: bridge: panel: allow override of the bus format

[Peter Rosin]

Useful if the bridge does some kind of conversion of the bus format.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/bridge/panel.c | 22 +++++++++++++++++++++-
 include/drm/drm_bridge.h       |  1 +
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index 6d99d4a3beb3..ccef0283ff41 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -22,6 +22,7 @@ struct panel_bridge {
 	struct drm_connector connector;
 	struct drm_panel *panel;
 	u32 connector_type;
+	u32 bus_format;
 };
 
 static inline struct panel_bridge *
@@ -40,8 +41,15 @@ static int panel_bridge_connector_get_modes(struct drm_connector *connector)
 {
 	struct panel_bridge *panel_bridge =
 		drm_connector_to_panel_bridge(connector);
+	int ret;
+
+	ret = drm_panel_get_modes(panel_bridge->panel);
+
+	if (panel_bridge->bus_format)
+		drm_display_info_set_bus_formats(&connector->display_info,
+						 &panel_bridge->bus_format, 1);
 
-	return drm_panel_get_modes(panel_bridge->panel);
+	return ret;
 }
 
 static const struct drm_connector_helper_funcs
@@ -203,6 +211,18 @@ void drm_panel_bridge_remove(struct drm_bridge *bridge)
 }
 EXPORT_SYMBOL(drm_panel_bridge_remove);
 
+void drm_panel_bridge_set_bus_format(struct drm_bridge *bridge, u32 bus_format)
+{
+	struct panel_bridge *panel_bridge;
+
+	if (!bridge)
+		return;
+
+	panel_bridge = drm_bridge_to_panel_bridge(bridge);
+	panel_bridge->bus_format = bus_format;
+}
+EXPORT_SYMBOL(drm_panel_bridge_set_bus_format);
+
 static void devm_drm_panel_bridge_release(struct device *dev, void *res)
 {
 	struct drm_bridge **bridge = res;
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 682d01ba920c..81903b92f187 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -268,6 +268,7 @@ void drm_bridge_enable(struct drm_bridge *bridge);
 struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel,
 					u32 connector_type);
 void drm_panel_bridge_remove(struct drm_bridge *bridge);
+void drm_panel_bridge_set_bus_format(struct drm_bridge *bridge, u32 bus_format);
 struct drm_bridge *devm_drm_panel_bridge_add(struct device *dev,
 					     struct drm_panel *panel,
 					     u32 connector_type);
-- 
2.11.0

Re: [RFC PATCH 2/3] drm: bridge: panel: allow override of the bus format

[Laurent Pinchart]

Hi Peter,

Thank you for the patch.

On Sunday, 18 March 2018 00:15:24 EET Peter Rosin wrote:
> Useful if the bridge does some kind of conversion of the bus format.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  drivers/gpu/drm/bridge/panel.c | 22 +++++++++++++++++++++-
>  include/drm/drm_bridge.h       |  1 +
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
> index 6d99d4a3beb3..ccef0283ff41 100644
> --- a/drivers/gpu/drm/bridge/panel.c
> +++ b/drivers/gpu/drm/bridge/panel.c
> @@ -22,6 +22,7 @@ struct panel_bridge {
>  	struct drm_connector connector;
>  	struct drm_panel *panel;
>  	u32 connector_type;
> +	u32 bus_format;
>  };
> 
>  static inline struct panel_bridge *
> @@ -40,8 +41,15 @@ static int panel_bridge_connector_get_modes(struct
> drm_connector *connector) {
>  	struct panel_bridge *panel_bridge =
>  		drm_connector_to_panel_bridge(connector);
> +	int ret;
> +
> +	ret = drm_panel_get_modes(panel_bridge->panel);
> +
> +	if (panel_bridge->bus_format)
> +		drm_display_info_set_bus_formats(&connector->display_info,
> +						 &panel_bridge->bus_format, 1);

While I agree with the problem statement and, to some extent, the DT bindings, 
I don't think this is the right implementation. You've correctly noted that 
display controller shouldn't blindly use the formats reported by the panel 
through the connector formats, and that hacking the panel driver to override 
the formats isn't a good idea, so I wouldn't override the formats reported by 
the connector. I would instead extend the drm_bridge API to report formats at 
bridge inputs. This would be more generic and allow each bridge to configure 
itself according to the next bridge in the chain.

I'm not sure whether this API extension should be in the form of a new bridge 
function, or if the formats should be stored in the drm_bridge structure 
directly as done for connectors in the display info structure. I'm tempted by 
the former, but I'm open to discussions.

> -	return drm_panel_get_modes(panel_bridge->panel);
> +	return ret;
>  }
> 
>  static const struct drm_connector_helper_funcs
> @@ -203,6 +211,18 @@ void drm_panel_bridge_remove(struct drm_bridge *bridge)
> }
>  EXPORT_SYMBOL(drm_panel_bridge_remove);
> 
> +void drm_panel_bridge_set_bus_format(struct drm_bridge *bridge, u32
> bus_format) +{
> +	struct panel_bridge *panel_bridge;
> +
> +	if (!bridge)
> +		return;
> +
> +	panel_bridge = drm_bridge_to_panel_bridge(bridge);
> +	panel_bridge->bus_format = bus_format;
> +}
> +EXPORT_SYMBOL(drm_panel_bridge_set_bus_format);
> +
>  static void devm_drm_panel_bridge_release(struct device *dev, void *res)
>  {
>  	struct drm_bridge **bridge = res;
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 682d01ba920c..81903b92f187 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -268,6 +268,7 @@ void drm_bridge_enable(struct drm_bridge *bridge);
>  struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel,
>  					u32 connector_type);
>  void drm_panel_bridge_remove(struct drm_bridge *bridge);
> +void drm_panel_bridge_set_bus_format(struct drm_bridge *bridge, u32
> bus_format);
> struct drm_bridge *devm_drm_panel_bridge_add(struct device
> *dev,
>  					     struct drm_panel *panel,
>  					     u32 connector_type);

-- 
Regards,

Laurent Pinchart

Re: [RFC PATCH 2/3] drm: bridge: panel: allow override of the bus format

[Peter Rosin]

Hi Laurent,

Thanks for the feedback!

On 2018-03-20 14:56, Laurent Pinchart wrote:
> Hi Peter,
> 
> Thank you for the patch.
> 
> On Sunday, 18 March 2018 00:15:24 EET Peter Rosin wrote:
>> Useful if the bridge does some kind of conversion of the bus format.
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
>> ---
>>  drivers/gpu/drm/bridge/panel.c | 22 +++++++++++++++++++++-
>>  include/drm/drm_bridge.h       |  1 +
>>  2 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
>> index 6d99d4a3beb3..ccef0283ff41 100644
>> --- a/drivers/gpu/drm/bridge/panel.c
>> +++ b/drivers/gpu/drm/bridge/panel.c
>> @@ -22,6 +22,7 @@ struct panel_bridge {
>>  	struct drm_connector connector;
>>  	struct drm_panel *panel;
>>  	u32 connector_type;
>> +	u32 bus_format;
>>  };
>>
>>  static inline struct panel_bridge *
>> @@ -40,8 +41,15 @@ static int panel_bridge_connector_get_modes(struct
>> drm_connector *connector) {
>>  	struct panel_bridge *panel_bridge =
>>  		drm_connector_to_panel_bridge(connector);
>> +	int ret;
>> +
>> +	ret = drm_panel_get_modes(panel_bridge->panel);
>> +
>> +	if (panel_bridge->bus_format)
>> +		drm_display_info_set_bus_formats(&connector->display_info,
>> +						 &panel_bridge->bus_format, 1);
> 
> While I agree with the problem statement and, to some extent, the DT bindings, 
> I don't think this is the right implementation. You've correctly noted that 
> display controller shouldn't blindly use the formats reported by the panel 
> through the connector formats, and that hacking the panel driver to override 
> the formats isn't a good idea, so I wouldn't override the formats reported by 
> the connector. I would instead extend the drm_bridge API to report formats at 
> bridge inputs. This would be more generic and allow each bridge to configure 
> itself according to the next bridge in the chain.
> 
> I'm not sure whether this API extension should be in the form of a new bridge 
> function, or if the formats should be stored in the drm_bridge structure 
> directly as done for connectors in the display info structure. I'm tempted by 
> the former, but I'm open to discussions.

Ok, I can look into that, but let me check if I got this right. From the very
little of the code that I have looked at, I have gathered that display
controllers handle bridges explicitly, right? If so, by extending the bridge
(with either a new function or new data) you impose changes to all display
controllers wanting to handle this new bridge input-format. If so, I assume
I can leave out the changes to all display controllers that I do not care
about. Correct?

Also, don't hold your breath waiting for a v2, but I'll try to get to it :-)

>> -	return drm_panel_get_modes(panel_bridge->panel);
>> +	return ret;
>>  }
>>
>>  static const struct drm_connector_helper_funcs
>> @@ -203,6 +211,18 @@ void drm_panel_bridge_remove(struct drm_bridge *bridge)
>> }
>>  EXPORT_SYMBOL(drm_panel_bridge_remove);
>>
>> +void drm_panel_bridge_set_bus_format(struct drm_bridge *bridge, u32
>> bus_format) +{
>> +	struct panel_bridge *panel_bridge;
>> +
>> +	if (!bridge)
>> +		return;
>> +
>> +	panel_bridge = drm_bridge_to_panel_bridge(bridge);
>> +	panel_bridge->bus_format = bus_format;
>> +}
>> +EXPORT_SYMBOL(drm_panel_bridge_set_bus_format);
>> +
>>  static void devm_drm_panel_bridge_release(struct device *dev, void *res)
>>  {
>>  	struct drm_bridge **bridge = res;
>> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
>> index 682d01ba920c..81903b92f187 100644
>> --- a/include/drm/drm_bridge.h
>> +++ b/include/drm/drm_bridge.h
>> @@ -268,6 +268,7 @@ void drm_bridge_enable(struct drm_bridge *bridge);
>>  struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel,
>>  					u32 connector_type);
>>  void drm_panel_bridge_remove(struct drm_bridge *bridge);
>> +void drm_panel_bridge_set_bus_format(struct drm_bridge *bridge, u32
>> bus_format);
>> struct drm_bridge *devm_drm_panel_bridge_add(struct device
>> *dev,
>>  					     struct drm_panel *panel,
>>  					     u32 connector_type);
> 

Re: [RFC PATCH 2/3] drm: bridge: panel: allow override of the bus format

[Laurent Pinchart]

Hi Peter,

(CC'ing Jacopo Mondi)

On Sunday, 25 March 2018 15:01:11 EEST Peter Rosin wrote:
> On 2018-03-20 14:56, Laurent Pinchart wrote:
> > Hi Peter,
> > 
> > Thank you for the patch.
> > 
> > On Sunday, 18 March 2018 00:15:24 EET Peter Rosin wrote:
> >> Useful if the bridge does some kind of conversion of the bus format.
> >> 
> >> Signed-off-by: Peter Rosin <peda@axentia.se>
> >> ---
> >> 
> >>  drivers/gpu/drm/bridge/panel.c | 22 +++++++++++++++++++++-
> >>  include/drm/drm_bridge.h       |  1 +
> >>  2 files changed, 22 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/gpu/drm/bridge/panel.c
> >> b/drivers/gpu/drm/bridge/panel.c index 6d99d4a3beb3..ccef0283ff41 100644
> >> --- a/drivers/gpu/drm/bridge/panel.c
> >> +++ b/drivers/gpu/drm/bridge/panel.c
> >> @@ -22,6 +22,7 @@ struct panel_bridge {
> >>  	struct drm_connector connector;
> >>  	struct drm_panel *panel;
> >>  	u32 connector_type;
> >> +	u32 bus_format;
> >>  };
> >>  
> >>  static inline struct panel_bridge *
> >> @@ -40,8 +41,15 @@ static int panel_bridge_connector_get_modes(struct
> >> drm_connector *connector) {
> >>  	struct panel_bridge *panel_bridge =
> >>  		drm_connector_to_panel_bridge(connector);
> >> 
> >> +	int ret;
> >> +
> >> +	ret = drm_panel_get_modes(panel_bridge->panel);
> >> +
> >> +	if (panel_bridge->bus_format)
> >> +		drm_display_info_set_bus_formats(&connector->display_info,
> >> +						 &panel_bridge->bus_format, 1);
> > 
> > While I agree with the problem statement and, to some extent, the DT
> > bindings, I don't think this is the right implementation. You've
> > correctly noted that display controller shouldn't blindly use the formats
> > reported by the panel through the connector formats, and that hacking the
> > panel driver to override the formats isn't a good idea, so I wouldn't
> > override the formats reported by the connector. I would instead extend
> > the drm_bridge API to report formats at bridge inputs. This would be more
> > generic and allow each bridge to configure itself according to the next
> > bridge in the chain.
> > 
> > I'm not sure whether this API extension should be in the form of a new
> > bridge function, or if the formats should be stored in the drm_bridge
> > structure directly as done for connectors in the display info structure.
> > I'm tempted by the former, but I'm open to discussions.
> 
> Ok, I can look into that, but let me check if I got this right. From the
> very little of the code that I have looked at, I have gathered that display
> controllers handle bridges explicitly, right?

That's correct, yes. Or, rather, they handle the first bridge in the chain, 
and then other bridges are handled recursively.

> If so, by extending the bridge (with either a new function or new data) you
> impose changes to all display controllers wanting to handle this new bridge
> input-format. If so, I assume I can leave out the changes to all display
> controllers that I do not care about. Correct?

That's correct.

> Also, don't hold your breath waiting for a v2, but I'll try to get to it :-)

I won't hold my breath, but Jacopo might :-) He has a similar issue to solve 
(reporting the LVDS modes supported by the bridge).

> >> -	return drm_panel_get_modes(panel_bridge->panel);
> >> +	return ret;
> >>  }
> >>  
> >>  static const struct drm_connector_helper_funcs
> >> @@ -203,6 +211,18 @@ void drm_panel_bridge_remove(struct drm_bridge
> >> *bridge)
> >>  }
> >>  EXPORT_SYMBOL(drm_panel_bridge_remove);
> >> 
> >> +void drm_panel_bridge_set_bus_format(struct drm_bridge *bridge, u32
> >> bus_format)
> >> +{
> >> +	struct panel_bridge *panel_bridge;
> >> +
> >> +	if (!bridge)
> >> +		return;
> >> +
> >> +	panel_bridge = drm_bridge_to_panel_bridge(bridge);
> >> +	panel_bridge->bus_format = bus_format;
> >> +}
> >> +EXPORT_SYMBOL(drm_panel_bridge_set_bus_format);
> >> +
> >>  static void devm_drm_panel_bridge_release(struct device *dev, void *res)
> >>  {
> >>  	struct drm_bridge **bridge = res;
> >> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> >> index 682d01ba920c..81903b92f187 100644
> >> --- a/include/drm/drm_bridge.h
> >> +++ b/include/drm/drm_bridge.h
> >> @@ -268,6 +268,7 @@ void drm_bridge_enable(struct drm_bridge *bridge);
> >>  struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel,
> >>  					u32 connector_type);
> >>  void drm_panel_bridge_remove(struct drm_bridge *bridge);
> >> +void drm_panel_bridge_set_bus_format(struct drm_bridge *bridge, u32
> >> bus_format);
> >> struct drm_bridge *devm_drm_panel_bridge_add(struct device *dev,
> >>  					     struct drm_panel *panel,
> >>  					     u32 connector_type);

-- 
Regards,

Laurent Pinchart

Re: [RFC PATCH 2/3] drm: bridge: panel: allow override of the bus format

[jacopo mondi]

[-- Attachment #1: Type: text/plain, Size: 6599 bytes --]

HI Laurent, Peter,

On Mon, Mar 26, 2018 at 10:03:31PM +0300, Laurent Pinchart wrote:
> Hi Peter,
>
> (CC'ing Jacopo Mondi)
>
> On Sunday, 25 March 2018 15:01:11 EEST Peter Rosin wrote:
> > On 2018-03-20 14:56, Laurent Pinchart wrote:
> > > Hi Peter,
> > >
> > > Thank you for the patch.
> > >
> > > On Sunday, 18 March 2018 00:15:24 EET Peter Rosin wrote:
> > >> Useful if the bridge does some kind of conversion of the bus format.
> > >>
> > >> Signed-off-by: Peter Rosin <peda@axentia.se>
> > >> ---
> > >>
> > >>  drivers/gpu/drm/bridge/panel.c | 22 +++++++++++++++++++++-
> > >>  include/drm/drm_bridge.h       |  1 +
> > >>  2 files changed, 22 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/bridge/panel.c
> > >> b/drivers/gpu/drm/bridge/panel.c index 6d99d4a3beb3..ccef0283ff41 100644
> > >> --- a/drivers/gpu/drm/bridge/panel.c
> > >> +++ b/drivers/gpu/drm/bridge/panel.c
> > >> @@ -22,6 +22,7 @@ struct panel_bridge {
> > >>  	struct drm_connector connector;
> > >>  	struct drm_panel *panel;
> > >>  	u32 connector_type;
> > >> +	u32 bus_format;
> > >>  };
> > >>
> > >>  static inline struct panel_bridge *
> > >> @@ -40,8 +41,15 @@ static int panel_bridge_connector_get_modes(struct
> > >> drm_connector *connector) {
> > >>  	struct panel_bridge *panel_bridge =
> > >>  		drm_connector_to_panel_bridge(connector);
> > >>
> > >> +	int ret;
> > >> +
> > >> +	ret = drm_panel_get_modes(panel_bridge->panel);
> > >> +
> > >> +	if (panel_bridge->bus_format)
> > >> +		drm_display_info_set_bus_formats(&connector->display_info,
> > >> +						 &panel_bridge->bus_format, 1);
> > >
> > > While I agree with the problem statement and, to some extent, the DT
> > > bindings, I don't think this is the right implementation. You've
> > > correctly noted that display controller shouldn't blindly use the formats
> > > reported by the panel through the connector formats, and that hacking the
> > > panel driver to override the formats isn't a good idea, so I wouldn't
> > > override the formats reported by the connector. I would instead extend
> > > the drm_bridge API to report formats at bridge inputs. This would be more
> > > generic and allow each bridge to configure itself according to the next
> > > bridge in the chain.
> > >
> > > I'm not sure whether this API extension should be in the form of a new
> > > bridge function, or if the formats should be stored in the drm_bridge
> > > structure directly as done for connectors in the display info structure.
> > > I'm tempted by the former, but I'm open to discussions.
> >
> > Ok, I can look into that, but let me check if I got this right. From the
> > very little of the code that I have looked at, I have gathered that display
> > controllers handle bridges explicitly, right?
>
> That's correct, yes. Or, rather, they handle the first bridge in the chain,
> and then other bridges are handled recursively.
>
> > If so, by extending the bridge (with either a new function or new data) you
> > impose changes to all display controllers wanting to handle this new bridge
> > input-format. If so, I assume I can leave out the changes to all display
> > controllers that I do not care about. Correct?
>
> That's correct.
>
> > Also, don't hold your breath waiting for a v2, but I'll try to get to it :-)
>
> I won't hold my breath, but Jacopo might :-) He has a similar issue to solve
> (reporting the LVDS modes supported by the bridge).
>

I was not :) I jumped late on this, as I restarted the DRM bridge work
yesterday. Peter, can I summarize you my current use case? (which is
quite similar to yours actually)

At the R-Car DU (Display Unit) output, we have an LVDS encoder
connected to a 'transparent' LVDS bridge that converts the input LVDS
stream into digital RGB output to be then HDMI encoded by another
component.

The 'transparent LVDS decoder', for which I'm now writing a driver,
should report what pixel bus format it accepts as input as the DU LVDS
encoder can output an LVDS stream with several different component ordering
schemes. I was about to come up with a proposal last week but you beat
me at time, so I'm happy to base my work on what comes out from this
series.

---- Laurent: On the THC63LVD driver
Laurent: at the same time I would not block the THC63LVD1024 driver. I
can extend bindings to include the "mode map" configuration property,
and squash my Eagle DTS patch on top of Niklas' one. Then make the newly
introduced driver use whatever API comes out from this series with an
incremental patch. Does this work for you? It is true, though, that
we're anyway late for v4.17 and I could send one single series based
on some future version of this and that includes all (bridge driver
and bindings, DU LVDS changes and Eagle DTS), but I feel it's easier
if we got the bridge driver accepted first, and then develop on top of
that.

Thanks
   j

> > >> -	return drm_panel_get_modes(panel_bridge->panel);
> > >> +	return ret;
> > >>  }
> > >>
> > >>  static const struct drm_connector_helper_funcs
> > >> @@ -203,6 +211,18 @@ void drm_panel_bridge_remove(struct drm_bridge
> > >> *bridge)
> > >>  }
> > >>  EXPORT_SYMBOL(drm_panel_bridge_remove);
> > >>
> > >> +void drm_panel_bridge_set_bus_format(struct drm_bridge *bridge, u32
> > >> bus_format)
> > >> +{
> > >> +	struct panel_bridge *panel_bridge;
> > >> +
> > >> +	if (!bridge)
> > >> +		return;
> > >> +
> > >> +	panel_bridge = drm_bridge_to_panel_bridge(bridge);
> > >> +	panel_bridge->bus_format = bus_format;
> > >> +}
> > >> +EXPORT_SYMBOL(drm_panel_bridge_set_bus_format);
> > >> +
> > >>  static void devm_drm_panel_bridge_release(struct device *dev, void *res)
> > >>  {
> > >>  	struct drm_bridge **bridge = res;
> > >> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > >> index 682d01ba920c..81903b92f187 100644
> > >> --- a/include/drm/drm_bridge.h
> > >> +++ b/include/drm/drm_bridge.h
> > >> @@ -268,6 +268,7 @@ void drm_bridge_enable(struct drm_bridge *bridge);
> > >>  struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel,
> > >>  					u32 connector_type);
> > >>  void drm_panel_bridge_remove(struct drm_bridge *bridge);
> > >> +void drm_panel_bridge_set_bus_format(struct drm_bridge *bridge, u32
> > >> bus_format);
> > >> struct drm_bridge *devm_drm_panel_bridge_add(struct device *dev,
> > >>  					     struct drm_panel *panel,
> > >>  					     u32 connector_type);
>
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC PATCH 2/3] drm: bridge: panel: allow override of the bus format

[Laurent Pinchart]

Hi Jacopo,

On Tuesday, 27 March 2018 11:16:01 EEST jacopo mondi wrote:
> On Mon, Mar 26, 2018 at 10:03:31PM +0300, Laurent Pinchart wrote:
> > On Sunday, 25 March 2018 15:01:11 EEST Peter Rosin wrote:
> >> On 2018-03-20 14:56, Laurent Pinchart wrote:
> >>> On Sunday, 18 March 2018 00:15:24 EET Peter Rosin wrote:
> >>>> Useful if the bridge does some kind of conversion of the bus format.
> >>>> 
> >>>> Signed-off-by: Peter Rosin <peda@axentia.se>
> >>>> ---
> >>>> 
> >>>>  drivers/gpu/drm/bridge/panel.c | 22 +++++++++++++++++++++-
> >>>>  include/drm/drm_bridge.h       |  1 +
> >>>>  2 files changed, 22 insertions(+), 1 deletion(-)
> >>>> 
> >>>> diff --git a/drivers/gpu/drm/bridge/panel.c
> >>>> b/drivers/gpu/drm/bridge/panel.c index 6d99d4a3beb3..ccef0283ff41
> >>>> 100644
> >>>> --- a/drivers/gpu/drm/bridge/panel.c
> >>>> +++ b/drivers/gpu/drm/bridge/panel.c
> >>>> @@ -22,6 +22,7 @@ struct panel_bridge {
> >>>>  	struct drm_connector connector;
> >>>>  	struct drm_panel *panel;
> >>>>  	u32 connector_type;
> >>>> +	u32 bus_format;
> >>>>  };
> >>>>  
> >>>>  static inline struct panel_bridge *
> >>>> @@ -40,8 +41,15 @@ static int panel_bridge_connector_get_modes(struct
> >>>> drm_connector *connector) {
> >>>>  	struct panel_bridge *panel_bridge =
> >>>>  		drm_connector_to_panel_bridge(connector);
> >>>> +	int ret;
> >>>> +
> >>>> +	ret = drm_panel_get_modes(panel_bridge->panel);
> >>>> +
> >>>> +	if (panel_bridge->bus_format)
> >>>> +		drm_display_info_set_bus_formats(&connector->display_info,
> >>>> +						 &panel_bridge->bus_format, 1);
> >>> 
> >>> While I agree with the problem statement and, to some extent, the DT
> >>> bindings, I don't think this is the right implementation. You've
> >>> correctly noted that display controller shouldn't blindly use the
> >>> formats reported by the panel through the connector formats, and that
> >>> hacking the panel driver to override the formats isn't a good idea, so
> >>> I wouldn't override the formats reported by the connector. I would
> >>> instead extend the drm_bridge API to report formats at bridge inputs.
> >>> This would be more generic and allow each bridge to configure itself
> >>> according to the next bridge in the chain.
> >>> 
> >>> I'm not sure whether this API extension should be in the form of a new
> >>> bridge function, or if the formats should be stored in the drm_bridge
> >>> structure directly as done for connectors in the display info
> >>> structure. I'm tempted by the former, but I'm open to discussions.
> >> 
> >> Ok, I can look into that, but let me check if I got this right. From the
> >> very little of the code that I have looked at, I have gathered that
> >> display controllers handle bridges explicitly, right?
> > 
> > That's correct, yes. Or, rather, they handle the first bridge in the
> > chain, and then other bridges are handled recursively.
> > 
> >> If so, by extending the bridge (with either a new function or new data)
> >> you impose changes to all display controllers wanting to handle this new
> >> bridge input-format. If so, I assume I can leave out the changes to all
> >> display controllers that I do not care about. Correct?
> > 
> > That's correct.
> > 
> >> Also, don't hold your breath waiting for a v2, but I'll try to get to it
> >> :-)
> > 
> > I won't hold my breath, but Jacopo might :-) He has a similar issue to
> > solve (reporting the LVDS modes supported by the bridge).
> 
> I was not :) I jumped late on this, as I restarted the DRM bridge work
> yesterday. Peter, can I summarize you my current use case? (which is
> quite similar to yours actually)
> 
> At the R-Car DU (Display Unit) output, we have an LVDS encoder
> connected to a 'transparent' LVDS bridge that converts the input LVDS
> stream into digital RGB output to be then HDMI encoded by another
> component.
> 
> The 'transparent LVDS decoder', for which I'm now writing a driver,
> should report what pixel bus format it accepts as input as the DU LVDS
> encoder can output an LVDS stream with several different component ordering
> schemes. I was about to come up with a proposal last week but you beat
> me at time, so I'm happy to base my work on what comes out from this
> series.
> 
> ---- Laurent: On the THC63LVD driver
> Laurent: at the same time I would not block the THC63LVD1024 driver. I
> can extend bindings to include the "mode map" configuration property,
> and squash my Eagle DTS patch on top of Niklas' one. Then make the newly
> introduced driver use whatever API comes out from this series with an
> incremental patch. Does this work for you? It is true, though, that
> we're anyway late for v4.17 and I could send one single series based
> on some future version of this and that includes all (bridge driver
> and bindings, DU LVDS changes and Eagle DTS), but I feel it's easier
> if we got the bridge driver accepted first, and then develop on top of
> that.

I'm fine with both options, you can pick the one that will match what is ready 
in upstream by the time you get to submit the patches.

> >>>> -	return drm_panel_get_modes(panel_bridge->panel);
> >>>> +	return ret;
> >>>>  }
> >>>>  
> >>>>  static const struct drm_connector_helper_funcs
> >>>> @@ -203,6 +211,18 @@ void drm_panel_bridge_remove(struct drm_bridge
> >>>> *bridge)
> >>>>  }
> >>>>  EXPORT_SYMBOL(drm_panel_bridge_remove);
> >>>> 
> >>>> +void drm_panel_bridge_set_bus_format(struct drm_bridge *bridge, u32
> >>>> bus_format)
> >>>> +{
> >>>> +	struct panel_bridge *panel_bridge;
> >>>> +
> >>>> +	if (!bridge)
> >>>> +		return;
> >>>> +
> >>>> +	panel_bridge = drm_bridge_to_panel_bridge(bridge);
> >>>> +	panel_bridge->bus_format = bus_format;
> >>>> +}
> >>>> +EXPORT_SYMBOL(drm_panel_bridge_set_bus_format);
> >>>> +
> >>>>  static void devm_drm_panel_bridge_release(struct device *dev, void
> >>>>  *res)
> >>>>  {
> >>>>  	struct drm_bridge **bridge = res;
> >>>> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> >>>> index 682d01ba920c..81903b92f187 100644
> >>>> --- a/include/drm/drm_bridge.h
> >>>> +++ b/include/drm/drm_bridge.h
> >>>> @@ -268,6 +268,7 @@ void drm_bridge_enable(struct drm_bridge
> >>>> *bridge);
> >>>>  struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel,
> >>>>  					u32 connector_type);
> >>>>  void drm_panel_bridge_remove(struct drm_bridge *bridge);
> >>>> +void drm_panel_bridge_set_bus_format(struct drm_bridge *bridge, u32
> >>>> bus_format);
> >>>> struct drm_bridge *devm_drm_panel_bridge_add(struct device *dev,
> >>>>  					     struct drm_panel *panel,
> >>>>  					     u32 connector_type);

-- 
Regards,

Laurent Pinchart

[RFC PATCH 3/3] drm: bridge: lvds-encoder: on request, override the bus format

[Peter Rosin]

If the bridge changes the bus format, allow this to be described in
the bridge, instead of providing false information about the bus
format of the panel itself.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../bindings/display/bridge/lvds-transmitter.txt       |  8 ++++++++
 drivers/gpu/drm/bridge/lvds-encoder.c                  | 18 ++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
index 9d09190d9210..c0fbe74272e7 100644
--- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
+++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
@@ -29,6 +29,14 @@ Required properties:
 
 	      "ti,ds90c185"
 
+Optional properties:
+
+- interface-pix-fmt:
+	      Override the bus format of the panel, in case the bridge
+	      converts the signals. Recognized formats include:
+
+	      "rgb565"
+
 Required nodes:
 
 This device has two video ports. Their connections are modeled using the OF
diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c b/drivers/gpu/drm/bridge/lvds-encoder.c
index 75b0d3f6e4de..acff3a5b0562 100644
--- a/drivers/gpu/drm/bridge/lvds-encoder.c
+++ b/drivers/gpu/drm/bridge/lvds-encoder.c
@@ -39,6 +39,9 @@ static int lvds_encoder_probe(struct platform_device *pdev)
 	struct device_node *panel_node;
 	struct drm_panel *panel;
 	struct lvds_encoder *lvds_encoder;
+	u32 bus_format = 0;
+	const char *fmt;
+	int ret;
 
 	lvds_encoder = devm_kzalloc(&pdev->dev, sizeof(*lvds_encoder),
 				    GFP_KERNEL);
@@ -79,6 +82,21 @@ static int lvds_encoder_probe(struct platform_device *pdev)
 	if (IS_ERR(lvds_encoder->panel_bridge))
 		return PTR_ERR(lvds_encoder->panel_bridge);
 
+	ret = of_property_read_string(pdev->dev.of_node,
+				      "interface-pix-fmt", &fmt);
+	if (!ret) {
+		if (!strcmp(fmt, "rgb565")) {
+			bus_format = MEDIA_BUS_FMT_RGB565_1X16;
+		} else {
+			dev_dbg(&pdev->dev,
+				"requested interface-pix-fmt not recognized\n");
+			return -EINVAL;
+		}
+	}
+	if (bus_format)
+		drm_panel_bridge_set_bus_format(lvds_encoder->panel_bridge,
+						bus_format);
+
 	/* The panel_bridge bridge is attached to the panel's of_node,
 	 * but we need a bridge attached to our of_node for our user
 	 * to look up.
-- 
2.11.0

Re: [RFC PATCH 3/3] drm: bridge: lvds-encoder: on request, override the bus format

[Laurent Pinchart]

Hi Peter,

Thank you for the patch.

On Sunday, 18 March 2018 00:15:25 EET Peter Rosin wrote:
> If the bridge changes the bus format, allow this to be described in
> the bridge, instead of providing false information about the bus
> format of the panel itself.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  .../bindings/display/bridge/lvds-transmitter.txt       |  8 ++++++++
>  drivers/gpu/drm/bridge/lvds-encoder.c                  | 18 +++++++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> index 9d09190d9210..c0fbe74272e7 100644
> --- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> @@ -29,6 +29,14 @@ Required properties:
> 
>  	      "ti,ds90c185"
> 
> +Optional properties:
> +
> +- interface-pix-fmt:
> +	      Override the bus format of the panel, in case the bridge
> +	      converts the signals. Recognized formats include:
> +
> +	      "rgb565"

Please describe the format in more details, with an explicit mapping of bits 
to signals, otherwise we'll end up again with multiple different 
interpretations (at least when it comes to endianness).

> +
>  Required nodes:
> 
>  This device has two video ports. Their connections are modeled using the OF
> diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c
> b/drivers/gpu/drm/bridge/lvds-encoder.c index 75b0d3f6e4de..acff3a5b0562
> 100644
> --- a/drivers/gpu/drm/bridge/lvds-encoder.c
> +++ b/drivers/gpu/drm/bridge/lvds-encoder.c
> @@ -39,6 +39,9 @@ static int lvds_encoder_probe(struct platform_device
> *pdev) struct device_node *panel_node;
>  	struct drm_panel *panel;
>  	struct lvds_encoder *lvds_encoder;
> +	u32 bus_format = 0;
> +	const char *fmt;
> +	int ret;
> 
>  	lvds_encoder = devm_kzalloc(&pdev->dev, sizeof(*lvds_encoder),
>  				    GFP_KERNEL);
> @@ -79,6 +82,21 @@ static int lvds_encoder_probe(struct platform_device
> *pdev) if (IS_ERR(lvds_encoder->panel_bridge))
>  		return PTR_ERR(lvds_encoder->panel_bridge);
> 
> +	ret = of_property_read_string(pdev->dev.of_node,
> +				      "interface-pix-fmt", &fmt);
> +	if (!ret) {
> +		if (!strcmp(fmt, "rgb565")) {
> +			bus_format = MEDIA_BUS_FMT_RGB565_1X16;

I think the string to fourcc conversion should take place in the DRM core, not 
in the LVDS encoder driver. This would avoid ending up with multiple 
incompatible conversions. It might even be better to move the DT parsing to 
the core as well, not just the conversion.

> +		} else {
> +			dev_dbg(&pdev->dev,
> +				"requested interface-pix-fmt not recognized\n");
> +			return -EINVAL;
> +		}
> +	}
> +	if (bus_format)
> +		drm_panel_bridge_set_bus_format(lvds_encoder->panel_bridge,
> +						bus_format);
> +
>  	/* The panel_bridge bridge is attached to the panel's of_node,
>  	 * but we need a bridge attached to our of_node for our user
>  	 * to look up.

-- 
Regards,

Laurent Pinchart

Re: [RFC PATCH 0/3] allow override of bus format in bridges

[Laurent Pinchart]

Hi Peter,

(CC'ing Jacopo Mondi who might be working on something similar)

On Sunday, 18 March 2018 00:15:22 EET Peter Rosin wrote:
> I'm trying to get something to work that I assumed would not
> need patches, so I think I might be missing something completely
> obvious...
> 
> I have an Atmel sama5d31 hooked up to an lvds encoder and then
> on to an lvds panel. Which seems like something that has been
> done one or two times before...
> 
> The problem (I think) is that the bus_format of the SoC and the
> panel do not agree. The SoC driver (atmel-hlcdc) can handle the
> rgb444, rgb565, rgb666 and rgb888 bus formats. The hardware is
> wired for the rgb565 case. The lvds encoder supports rgb888 on
> its input side with the LSB wires simply pulled down internally
> in the encoder in my case. And the panel is expecting lvds
> (vesa-24), which is what the encoder outputs.
> 
> The reason I "blame" the bus_format of the drm_connector is that
> with the below DT snippet, things do not work *exactly* due to
> that. At least, it starts to work if I hack the panel-lvds driver
> to report the rgb565 bus_format instead of vesa-24.
> 
> 	panel: panel {
> 		compatible = "panel-lvds";
> 
> 		width-mm = <476>;
> 		height-mm = <268>;
> 
> 		data-mapping = "vesa-24";
> 
> 		panel-timing {
> 			// 1024x768 @ 60Hz (typical)
> 			clock-frequency = <52140000 65000000 71100000>;
> 			hactive = <1024>;
> 			vactive = <768>;
> 			hfront-porch = <59 107 107>;
> 			hback-porch = <59 107 107>;
> 			hsync-len = <59 106 106>;
> 			vfront-porch = <8 13 14>;
> 			vback-porch = <8 13 14>;
> 			vsync-len = <8 12 14>;
> 		};
> 
> 		port {
> 			panel_input: endpoint {
> 				remote-endpoint = <&lvds_encoder_output>;
> 			};
> 		};
> 	};
> 
> 	lvds-encoder {
> 		compatible = "ti,ds90c187", "lvds-encoder";
> 
> 		ports {
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 
> 			port@0 {
> 				reg = <0>;
> 
> 				lvds_encoder_input: endpoint {
> 					remote-endpoint = <&hlcdc_output>;
> 				};
> 			};
> 
> 			port@1 {
> 				reg = <1>;
> 
> 				lvds_encoder_output: endpoint {
> 					remote-endpoint = <&panel_input>;
> 				};
> 			};
> 		};
> 	};

I agree with your analysis, it's wrong for a display controller to use the 
formats reported by the panel (through the connector) without taking into 
account intermediate bridges.

> But, instead of perverting the panel-lvds driver with support
> for a totally fake non-lvds bus_format, I came up with an optional
> bus_format override in the lvds-encoder driver, which I trigger with
> this addition to the lvds-encoder DT node:
> 
> 		interface-pix-fmt = "rgb565";
> 
> There are some issues with the interface-pix-fmt name. I copied it
> from the freescale fsl-imx-drm binding, but a) I personally don't
> like how that binding uses rgb24 instead of rgb888 and b) I don't
> like how the implementation of that binding selects rgb666 when you
> say bgr666 in the DT, but for parallel hardware interface the bit
> order is not really relevant so I swallowed that. Anyway, it might
> be better to use something else than interface-pix-fmt? Perhaps
> reuse the "data-mapping" name from the panel-lvds binding? But that
> seems somewhat lvds specific? Then there a couple of instances of
> "bus-format-override" in some dtsi files, but that name is not
> mentioned in any binding, nor in (upstream) code...

The data-mappings property describe the mapping of the bits over the LVDS time 
slots, so I wouldn't reuse that name, especially given that the property you 
need isn't specific to LVDS. "bus-format" sounds better to me, or maybe 
"video-format" (coming from a similar property in a Xilinx-specific V4L2 
binding).

> And maybe it should be fourcc codes or something instead of these
> "rgb565" names?

We'd end up introducing the format fourcc values, which are to some extent 
Linux-specific (with identical formats that have different fourccs in V4L2 and 
DRM), in the DT bindings, so I'm tempted to stick to strings, but I could be 
convinced otherwise.

Another option would be to use device-specific compatible strings, in which 
case the format could be stored in the driver instead of in a DT property. I'm 
not sure that's a good idea, I haven't really thought about it much :-)

> I threw in the first patch, since that is the actual lvds encoder
> I have in this case.
> 
> Suggestions welcome.

I'll comment on the API in replies to the patches.

> Peter Rosin (3):
>   dt-bindings: display: bridge: lvds-transmitter: add ti,ds90c185
>   drm: bridge: panel: allow override of the bus format
>   drm: bridge: lvds-encoder: on request, override the bus format
> 
>  .../bindings/display/bridge/lvds-transmitter.txt   | 13 +++++++++++++
>  drivers/gpu/drm/bridge/lvds-encoder.c              | 18 ++++++++++++++++++
>  drivers/gpu/drm/bridge/panel.c                     | 22 ++++++++++++++++++-
>  include/drm/drm_bridge.h                           |  1 +
>  4 files changed, 53 insertions(+), 1 deletion(-)

-- 
Regards,

Laurent Pinchart