[PATCH v1 0/6] drm/bridge: tc358768: Improve parallel RGB input configuration

[PATCH v1 0/6] drm/bridge: tc358768: Improve parallel RGB input configuration

[Francesco Dolcini]

From: Francesco Dolcini <francesco.dolcini@toradex.com>

Add support for different parallel RGB input format and mapping, this enables
using the TC358768 when the parallel input width is less than 24-bit or the
input mapping is not the default one.

In addition to that this series add support for the TC9594, that is the
automotive version of the TC358768 (100% identical from the SW point of view).

Francesco Dolcini (6):
  dt-bindings: display: bridge: toshiba,tc358768: Fix typo
  dt-bindings: display: bridge: toshiba,tc358768: Add TC9594
  dt-bindings: display: bridge: toshiba,tc358768: add parallel input
    mode
  drm/bridge: tc358768: Add TC9594
  drm/bridge: tc358768: Add parallel data format cfg
  drm/bridge: tc358768: Add atomic_get_input_bus_fmts() implementation

 .../display/bridge/toshiba,tc358768.yaml      | 20 +++++++-
 drivers/gpu/drm/bridge/tc358768.c             | 51 +++++++++++++++++++
 2 files changed, 69 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH v1 1/6] dt-bindings: display: bridge: toshiba,tc358768: Fix typo

[Francesco Dolcini]

From: Francesco Dolcini <francesco.dolcini@toradex.com>

Correct Toshiba spelling.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
 .../devicetree/bindings/display/bridge/toshiba,tc358768.yaml    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
index 0b6f5bef120f..d6dac186ac59 100644
--- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/display/bridge/toshiba,tc358768.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Toschiba TC358768/TC358778 Parallel RGB to MIPI DSI bridge
+title: Toshiba TC358768/TC358778 Parallel RGB to MIPI DSI bridge
 
 maintainers:
   - Peter Ujfalusi <peter.ujfalusi@ti.com>
-- 
2.25.1

Re: [PATCH v1 1/6] dt-bindings: display: bridge: toshiba,tc358768: Fix typo

[Krzysztof Kozlowski]

On 30/03/2023 11:59, Francesco Dolcini wrote:
> From: Francesco Dolcini <francesco.dolcini@toradex.com>
> 
> Correct Toshiba spelling.
> 
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH v1 2/6] dt-bindings: display: bridge: toshiba,tc358768: Add TC9594

[Francesco Dolcini]

From: Francesco Dolcini <francesco.dolcini@toradex.com>

Add TC9594, from the software point of view this is identical to
TC358768 with the main difference being automotive qualified.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
 .../devicetree/bindings/display/bridge/toshiba,tc358768.yaml | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
index d6dac186ac59..8f22093b61ae 100644
--- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
@@ -4,19 +4,20 @@
 $id: http://devicetree.org/schemas/display/bridge/toshiba,tc358768.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Toshiba TC358768/TC358778 Parallel RGB to MIPI DSI bridge
+title: Toshiba TC358768/TC358778/TC9594 Parallel RGB to MIPI DSI bridge
 
 maintainers:
   - Peter Ujfalusi <peter.ujfalusi@ti.com>
 
 description: |
-  The TC358768/TC358778 is bridge device which converts RGB to DSI.
+  The TC358768/TC358778/TC9594 is bridge device which converts RGB to DSI.
 
 properties:
   compatible:
     enum:
       - toshiba,tc358768
       - toshiba,tc358778
+      - toshiba,tc9594
 
   reg:
     maxItems: 1
-- 
2.25.1

Re: [PATCH v1 2/6] dt-bindings: display: bridge: toshiba,tc358768: Add TC9594

[Krzysztof Kozlowski]

On 30/03/2023 11:59, Francesco Dolcini wrote:
> From: Francesco Dolcini <francesco.dolcini@toradex.com>
> 
> Add TC9594, from the software point of view this is identical to
> TC358768 with the main difference being automotive qualified.
> 
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
>  .../devicetree/bindings/display/bridge/toshiba,tc358768.yaml | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> index d6dac186ac59..8f22093b61ae 100644
> --- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> @@ -4,19 +4,20 @@
>  $id: http://devicetree.org/schemas/display/bridge/toshiba,tc358768.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Toshiba TC358768/TC358778 Parallel RGB to MIPI DSI bridge
> +title: Toshiba TC358768/TC358778/TC9594 Parallel RGB to MIPI DSI bridge
>  
>  maintainers:
>    - Peter Ujfalusi <peter.ujfalusi@ti.com>
>  
>  description: |
> -  The TC358768/TC358778 is bridge device which converts RGB to DSI.
> +  The TC358768/TC358778/TC9594 is bridge device which converts RGB to DSI.
>  
>  properties:
>    compatible:
>      enum:
>        - toshiba,tc358768
>        - toshiba,tc358778
> +      - toshiba,tc9594

If it is the same, why they are not compatible? I got only three patches
out of six, thus I cannot check by myself. Please explain.

Best regards,
Krzysztof

Re: [PATCH v1 2/6] dt-bindings: display: bridge: toshiba,tc358768: Add TC9594

[Francesco Dolcini]

On Fri, Mar 31, 2023 at 10:42:40AM +0200, Krzysztof Kozlowski wrote:
> On 30/03/2023 11:59, Francesco Dolcini wrote:
> > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> > 
> > Add TC9594, from the software point of view this is identical to
> > TC358768 with the main difference being automotive qualified.
> > 
> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > ---
> >  .../devicetree/bindings/display/bridge/toshiba,tc358768.yaml | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> > index d6dac186ac59..8f22093b61ae 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> > @@ -4,19 +4,20 @@
> >  $id: http://devicetree.org/schemas/display/bridge/toshiba,tc358768.yaml#
> >  $schema: http://devicetree.org/meta-schemas/core.yaml#
> >  
> > -title: Toshiba TC358768/TC358778 Parallel RGB to MIPI DSI bridge
> > +title: Toshiba TC358768/TC358778/TC9594 Parallel RGB to MIPI DSI bridge
> >  
> >  maintainers:
> >    - Peter Ujfalusi <peter.ujfalusi@ti.com>
> >  
> >  description: |
> > -  The TC358768/TC358778 is bridge device which converts RGB to DSI.
> > +  The TC358768/TC358778/TC9594 is bridge device which converts RGB to DSI.
> >  
> >  properties:
> >    compatible:
> >      enum:
> >        - toshiba,tc358768
> >        - toshiba,tc358778
> > +      - toshiba,tc9594
> 
> If it is the same, why they are not compatible?

I assume I am not understanding something very basic ...

The register description and SW functionality of all these 3 parts is
identical.

How should be the compatible described in the DT schema in this case?


> I got only three patches out of six, thus I cannot check by myself.

Here the whole series: https://lore.kernel.org/all/20230330095941.428122-1-francesco@dolcini.it/

Do you want to me to send the whole series and not just the DT binding
next time? Happy to do it, if you prefer.

Francesco

Re: [PATCH v1 2/6] dt-bindings: display: bridge: toshiba,tc358768: Add TC9594

[Krzysztof Kozlowski]

On 31/03/2023 11:30, Francesco Dolcini wrote:
> On Fri, Mar 31, 2023 at 10:42:40AM +0200, Krzysztof Kozlowski wrote:
>> On 30/03/2023 11:59, Francesco Dolcini wrote:
>>> From: Francesco Dolcini <francesco.dolcini@toradex.com>
>>>
>>> Add TC9594, from the software point of view this is identical to
>>> TC358768 with the main difference being automotive qualified.
>>>
>>> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
>>> ---
>>>  .../devicetree/bindings/display/bridge/toshiba,tc358768.yaml | 5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
>>> index d6dac186ac59..8f22093b61ae 100644
>>> --- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
>>> +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
>>> @@ -4,19 +4,20 @@
>>>  $id: http://devicetree.org/schemas/display/bridge/toshiba,tc358768.yaml#
>>>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>>>  
>>> -title: Toshiba TC358768/TC358778 Parallel RGB to MIPI DSI bridge
>>> +title: Toshiba TC358768/TC358778/TC9594 Parallel RGB to MIPI DSI bridge
>>>  
>>>  maintainers:
>>>    - Peter Ujfalusi <peter.ujfalusi@ti.com>
>>>  
>>>  description: |
>>> -  The TC358768/TC358778 is bridge device which converts RGB to DSI.
>>> +  The TC358768/TC358778/TC9594 is bridge device which converts RGB to DSI.
>>>  
>>>  properties:
>>>    compatible:
>>>      enum:
>>>        - toshiba,tc358768
>>>        - toshiba,tc358778
>>> +      - toshiba,tc9594
>>
>> If it is the same, why they are not compatible?
> 
> I assume I am not understanding something very basic ...
> 
> The register description and SW functionality of all these 3 parts is
> identical.
> 
> How should be the compatible described in the DT schema in this case?

Look at any board compatibles for example.

> 
> 
>> I got only three patches out of six, thus I cannot check by myself.
> 
> Here the whole series: https://lore.kernel.org/all/20230330095941.428122-1-francesco@dolcini.it/
> 
> Do you want to me to send the whole series and not just the DT binding
> next time? Happy to do it, if you prefer.

So NAK on 4th patch. Don't grow device ID tables needlessly.

Best regards,
Krzysztof

[PATCH v1 3/6] dt-bindings: display: bridge: toshiba,tc358768: add parallel input mode

[Francesco Dolcini]

From: Francesco Dolcini <francesco.dolcini@toradex.com>

Add new toshiba,input-rgb-mode property to describe the actual signal
connection on the parallel RGB input interface.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
 .../bindings/display/bridge/toshiba,tc358768.yaml | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
index 8f22093b61ae..2638121a2223 100644
--- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
@@ -42,6 +42,21 @@ properties:
   clock-names:
     const: refclk
 
+  toshiba,input-rgb-mode:
+    description: |
+      Parallel Input (RGB) Mode.
+
+      RGB inputs (PD[23:0]) color arrangement as documented in the datasheet
+      and in the table below.
+
+      0 = R[7:0], G[7:0], B[7:0]
+      1 = R[1:0], G[1:0], B[1:0], R[7:2], G[7:2], B[7:2]
+      2 = 8’b0, R[4:0], G[5:0], B[4:0]
+
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [ 0, 1, 2 ]
+    default: 0
+
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
-- 
2.25.1

Re: [PATCH v1 3/6] dt-bindings: display: bridge: toshiba,tc358768: add parallel input mode

[Krzysztof Kozlowski]

On 30/03/2023 11:59, Francesco Dolcini wrote:
> From: Francesco Dolcini <francesco.dolcini@toradex.com>
> 
> Add new toshiba,input-rgb-mode property to describe the actual signal
> connection on the parallel RGB input interface.
> 
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
>  .../bindings/display/bridge/toshiba,tc358768.yaml | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> index 8f22093b61ae..2638121a2223 100644
> --- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> @@ -42,6 +42,21 @@ properties:
>    clock-names:
>      const: refclk
>  
> +  toshiba,input-rgb-mode:
> +    description: |
> +      Parallel Input (RGB) Mode.
> +
> +      RGB inputs (PD[23:0]) color arrangement as documented in the datasheet
> +      and in the table below.
> +
> +      0 = R[7:0], G[7:0], B[7:0]

RGB888?

> +      1 = R[1:0], G[1:0], B[1:0], R[7:2], G[7:2], B[7:2]
> +      2 = 8’b0, R[4:0], G[5:0], B[4:0]

Isn't this RGB565?

Don't we have already properties like this? e.g. colorspace?

Best regards,
Krzysztof

Re: [PATCH v1 3/6] dt-bindings: display: bridge: toshiba,tc358768: add parallel input mode

[Francesco Dolcini]

On Fri, Mar 31, 2023 at 10:48:15AM +0200, Krzysztof Kozlowski wrote:
> On 30/03/2023 11:59, Francesco Dolcini wrote:
> > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> > 
> > Add new toshiba,input-rgb-mode property to describe the actual signal
> > connection on the parallel RGB input interface.
> > 
> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > ---
> >  .../bindings/display/bridge/toshiba,tc358768.yaml | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> > index 8f22093b61ae..2638121a2223 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> > @@ -42,6 +42,21 @@ properties:
> >    clock-names:
> >      const: refclk
> >  
> > +  toshiba,input-rgb-mode:
> > +    description: |
> > +      Parallel Input (RGB) Mode.
> > +
> > +      RGB inputs (PD[23:0]) color arrangement as documented in the datasheet
> > +      and in the table below.
> > +
> > +      0 = R[7:0], G[7:0], B[7:0]
> 
> RGB888?

Or anything else - like a RGB666 - just connecting to GND the unused
pins.

> > +      1 = R[1:0], G[1:0], B[1:0], R[7:2], G[7:2], B[7:2]
> > +      2 = 8’b0, R[4:0], G[5:0], B[4:0]
> 
> Isn't this RGB565?
> 
> Don't we have already properties like this? e.g. colorspace?

It's not really the colorspace this property.

tc358768 is a parallel RGB to DSI bridge, it has 24 bit parallel input
line.

The way this lines are connected is configurable with this parameter, if you
look at mode 0 and 1 they all allow to have a RGB888 or a RGB666 or a
RGB565 mapping. This just configure some internal mux, it's not strictly
about the RGB mode.

The description for mode 2 was copied from the datasheet, and I agree
this is really about having a RGB565 on the first 16 parallel input
pins.

Honestly I do not know what is going to happen with PD[23-16] if they
are not connected to GND, given that the component does not know the
parallel input bus width it might as well sample those pins in some
un-documented way.

Francesco

Re: [PATCH v1 3/6] dt-bindings: display: bridge: toshiba,tc358768: add parallel input mode

[Rob Herring]

On Fri, Mar 31, 2023 at 11:40:01AM +0200, Francesco Dolcini wrote:
> On Fri, Mar 31, 2023 at 10:48:15AM +0200, Krzysztof Kozlowski wrote:
> > On 30/03/2023 11:59, Francesco Dolcini wrote:
> > > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > 
> > > Add new toshiba,input-rgb-mode property to describe the actual signal
> > > connection on the parallel RGB input interface.
> > > 
> > > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > ---
> > >  .../bindings/display/bridge/toshiba,tc358768.yaml | 15 +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> > > index 8f22093b61ae..2638121a2223 100644
> > > --- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> > > +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> > > @@ -42,6 +42,21 @@ properties:
> > >    clock-names:
> > >      const: refclk
> > >  
> > > +  toshiba,input-rgb-mode:
> > > +    description: |
> > > +      Parallel Input (RGB) Mode.
> > > +
> > > +      RGB inputs (PD[23:0]) color arrangement as documented in the datasheet
> > > +      and in the table below.
> > > +
> > > +      0 = R[7:0], G[7:0], B[7:0]
> > 
> > RGB888?
> 
> Or anything else - like a RGB666 - just connecting to GND the unused
> pins.

If the bridge is configured for RGB666, then that's fine. If not, the 
unused pins should be driven with either the MSB of each component. 
Otherwise, you'd can't fully saturate the colors.

> > > +      1 = R[1:0], G[1:0], B[1:0], R[7:2], G[7:2], B[7:2]
> > > +      2 = 8’b0, R[4:0], G[5:0], B[4:0]
> > 
> > Isn't this RGB565?
> > 
> > Don't we have already properties like this? e.g. colorspace?
> 
> It's not really the colorspace this property.
> 
> tc358768 is a parallel RGB to DSI bridge, it has 24 bit parallel input
> line.
> 
> The way this lines are connected is configurable with this parameter, if you
> look at mode 0 and 1 they all allow to have a RGB888 or a RGB666 or a
> RGB565 mapping. This just configure some internal mux, it's not strictly
> about the RGB mode.

This is the same as other cases. There's a need for describing the 
interface. It keeps coming up and I keep saying to go create something 
common.

Rob

Re: [PATCH v1 3/6] dt-bindings: display: bridge: toshiba,tc358768: add parallel input mode

[Francesco Dolcini]

On Mon, Apr 03, 2023 at 04:01:17PM -0500, Rob Herring wrote:
> On Fri, Mar 31, 2023 at 11:40:01AM +0200, Francesco Dolcini wrote:
> > On Fri, Mar 31, 2023 at 10:48:15AM +0200, Krzysztof Kozlowski wrote:
> > > On 30/03/2023 11:59, Francesco Dolcini wrote:
> > > > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > > 
> > > > Add new toshiba,input-rgb-mode property to describe the actual signal
> > > > connection on the parallel RGB input interface.
> > > > 
> > > > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > > ---
> > > >  .../bindings/display/bridge/toshiba,tc358768.yaml | 15 +++++++++++++++
> > > >  1 file changed, 15 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> > > > index 8f22093b61ae..2638121a2223 100644
> > > > --- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> > > > +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> > > > @@ -42,6 +42,21 @@ properties:
> > > >    clock-names:
> > > >      const: refclk
> > > >  
> > > > +  toshiba,input-rgb-mode:
> > > > +    description: |
> > > > +      Parallel Input (RGB) Mode.
> > > > +
> > > > +      RGB inputs (PD[23:0]) color arrangement as documented in the datasheet
> > > > +      and in the table below.
> > > > +
> > > > +      0 = R[7:0], G[7:0], B[7:0]
> > > 
> > > RGB888?
> > 
> > Or anything else - like a RGB666 - just connecting to GND the unused
> > pins.
> 
> If the bridge is configured for RGB666, then that's fine. If not, the 
> unused pins should be driven with either the MSB of each component. 
> Otherwise, you'd can't fully saturate the colors.

> > > > +      1 = R[1:0], G[1:0], B[1:0], R[7:2], G[7:2], B[7:2]
> > > > +      2 = 8’b0, R[4:0], G[5:0], B[4:0]
> > > 
> > > Isn't this RGB565?
> > > 
> > > Don't we have already properties like this? e.g. colorspace?
> > 
> > It's not really the colorspace this property.
> > 
> > tc358768 is a parallel RGB to DSI bridge, it has 24 bit parallel input
> > line.
> > 
> > The way this lines are connected is configurable with this parameter, if you
> > look at mode 0 and 1 they all allow to have a RGB888 or a RGB666 or a
> > RGB565 mapping. This just configure some internal mux, it's not strictly
> > about the RGB mode.
> 
> This is the same as other cases. There's a need for describing the 
> interface. It keeps coming up and I keep saying to go create something 
> common.

I am not aware of other discussion on the topic, do you have any pointer
I can look at?

What I'd like to re-iterate here once more is that this configuration is
about how the external 24-bit parallel RGB lines are mapped withing this
bridge.

It's not mapping the linux media bus format (e.g. not
MEDIA_BUS_FMT_RBG888_1X24 or alike).

This bridge allow for a limited set of combination (3) as described in
this binding.

Francesco

Re: [PATCH v1 3/6] dt-bindings: display: bridge: toshiba,tc358768: add parallel input mode

[Francesco Dolcini]

On Mon, Apr 03, 2023 at 04:01:17PM -0500, Rob Herring wrote:
> On Fri, Mar 31, 2023 at 11:40:01AM +0200, Francesco Dolcini wrote:
> > On Fri, Mar 31, 2023 at 10:48:15AM +0200, Krzysztof Kozlowski wrote:
> > > On 30/03/2023 11:59, Francesco Dolcini wrote:
> > > > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > > 
> > > > Add new toshiba,input-rgb-mode property to describe the actual signal
> > > > connection on the parallel RGB input interface.
> > > > 
> > > > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > > ---
> > > >  .../bindings/display/bridge/toshiba,tc358768.yaml | 15 +++++++++++++++
> > > >  1 file changed, 15 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> > > > index 8f22093b61ae..2638121a2223 100644
> > > > --- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> > > > +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> > > > @@ -42,6 +42,21 @@ properties:
> > > >    clock-names:
> > > >      const: refclk
> > > >  
> > > > +  toshiba,input-rgb-mode:
> > > > +    description: |
> > > > +      Parallel Input (RGB) Mode.
> > > > +
> > > > +      RGB inputs (PD[23:0]) color arrangement as documented in the datasheet
> > > > +      and in the table below.
> > > > +
> > > > +      0 = R[7:0], G[7:0], B[7:0]
> > > 
> > > RGB888?
> > 
> > Or anything else - like a RGB666 - just connecting to GND the unused
> > pins.
> 
> If the bridge is configured for RGB666, then that's fine. If not, the 
> unused pins should be driven with either the MSB of each component. 
> Otherwise, you'd can't fully saturate the colors.

maybe a detail and maybe not really relevant, but this specific bridge
has no know-how on the actual RGB inputs width. While I understand what
you are saying here, in the end this is about the actual hardware design
that can be in any way, including having pins to gnd and have the issue
you just described.

Francesco

Re: [PATCH v1 3/6] dt-bindings: display: bridge: toshiba,tc358768: add parallel input mode

[Francesco Dolcini]

Hello Rob and Krzysztof

On Mon, Apr 03, 2023 at 04:01:17PM -0500, Rob Herring wrote:
> On Fri, Mar 31, 2023 at 11:40:01AM +0200, Francesco Dolcini wrote:
> > On Fri, Mar 31, 2023 at 10:48:15AM +0200, Krzysztof Kozlowski wrote:
> > > On 30/03/2023 11:59, Francesco Dolcini wrote:
> > > > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > > 
> > > > Add new toshiba,input-rgb-mode property to describe the actual signal
> > > > connection on the parallel RGB input interface.
> > > > 
> > > > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > > ---
> > > >  .../bindings/display/bridge/toshiba,tc358768.yaml | 15 +++++++++++++++
> > > >  1 file changed, 15 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> > > > index 8f22093b61ae..2638121a2223 100644
> > > > --- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> > > > +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
> > > > @@ -42,6 +42,21 @@ properties:
> > > >    clock-names:
> > > >      const: refclk
> > > >  
> > > > +  toshiba,input-rgb-mode:
> > > > +    description: |
> > > > +      Parallel Input (RGB) Mode.
> > > > +
> > > > +      RGB inputs (PD[23:0]) color arrangement as documented in the datasheet
> > > > +      and in the table below.
> > > > +
> > > > +      0 = R[7:0], G[7:0], B[7:0]
> > tc358768 is a parallel RGB to DSI bridge, it has 24 bit parallel input
> > line.
> > 
> > The way this lines are connected is configurable with this parameter, if you
> > look at mode 0 and 1 they all allow to have a RGB888 or a RGB666 or a
> > RGB565 mapping. This just configure some internal mux, it's not strictly
> > about the RGB mode.
> 
> This is the same as other cases. There's a need for describing the 
> interface. It keeps coming up and I keep saying to go create something 
> common.

Rob, Krzysztof: you both had concerns on this change and I am not sure how
to progress. Let me summarize my current understanding.

1. I do not think that this change is related to the media bus format
   topic, this is more like the internal bridge RGB pins mux
   configuration. This `input-rgb-mode` field I am proposing here is
   orthogonal to the bus format (e.g. linux media-bus-format.h). I can
   change the name, if we think this is the reason of the confusion, I
   just used something similar to the words used in the datasheet.
   
   Do we have agreement on that or not?

2. Rob asked for a generic solution, since according to him this is a
   recurring need. I would appreciate some pointer where to look at so I
   can propose something more generic.

Thanks,
Francesco

[PATCH v1 4/6] drm/bridge: tc358768: Add TC9594

[Francesco Dolcini]

From: Francesco Dolcini <francesco.dolcini@toradex.com>

Add TC9594 ids, from the software point of view this is fully compatible
with tc358768, the only difference is the automotive qualification.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
 drivers/gpu/drm/bridge/tc358768.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
index 7c0cbe84611b..f4499ae7bee6 100644
--- a/drivers/gpu/drm/bridge/tc358768.c
+++ b/drivers/gpu/drm/bridge/tc358768.c
@@ -991,6 +991,7 @@ static const struct regmap_config tc358768_regmap_config = {
 static const struct i2c_device_id tc358768_i2c_ids[] = {
 	{ "tc358768", 0 },
 	{ "tc358778", 0 },
+	{ "tc9594", 0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, tc358768_i2c_ids);
@@ -998,6 +999,7 @@ MODULE_DEVICE_TABLE(i2c, tc358768_i2c_ids);
 static const struct of_device_id tc358768_of_ids[] = {
 	{ .compatible = "toshiba,tc358768", },
 	{ .compatible = "toshiba,tc358778", },
+	{ .compatible = "toshiba,tc9594", },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, tc358768_of_ids);
-- 
2.25.1

[PATCH v1 5/6] drm/bridge: tc358768: Add parallel data format cfg

[Francesco Dolcini]

From: Francesco Dolcini <francesco.dolcini@toradex.com>

Add configuration for parallel data format register, tc358768 supports
different mapping on the parallel input RGB interface, enable the
configuration for it.

Valid values, and the related meaning, are:
  0 = R[7:0], G[7:0], B[7:0]
  1 = R[1:0], G[1:0], B[1:0], R[7:2], G[7:2], B[7:2]
  2 = 8’b0, R[4:0], G[5:0], B[4:0]

Use 0 by default, consistently with the HW default.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
 drivers/gpu/drm/bridge/tc358768.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
index f4499ae7bee6..4462264274af 100644
--- a/drivers/gpu/drm/bridge/tc358768.c
+++ b/drivers/gpu/drm/bridge/tc358768.c
@@ -854,6 +854,11 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
 	if (mode->flags & DRM_MODE_FLAG_PHSYNC)
 		tc358768_update_bits(priv, TC358768_PP_MISC, BIT(0), BIT(0));
 
+	/* PDataF: Parallel Data Format */
+	val = 0;
+	of_property_read_u32(bridge->of_node, "toshiba,input-rgb-mode", &val);
+	tc358768_update_bits(priv, TC358768_CONFCTL, BIT(8) | BIT(9), val << 8);
+
 	/* Start DSI Tx */
 	tc358768_write(priv, TC358768_DSI_START, 0x1);
 
-- 
2.25.1

[PATCH v1 6/6] drm/bridge: tc358768: Add atomic_get_input_bus_fmts() implementation

[Francesco Dolcini]

From: Francesco Dolcini <francesco.dolcini@toradex.com>

Add atomic_get_input_bus_fmts() implementation, tc358768 has a parallel
RGB input interface with the actual bus format depending on the amount
of parallel input data lines.

Without this change when the tc358768 is used with less than 24bit the
color mapping is completely wrong.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
 drivers/gpu/drm/bridge/tc358768.c | 44 +++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
index 4462264274af..c41620409a39 100644
--- a/drivers/gpu/drm/bridge/tc358768.c
+++ b/drivers/gpu/drm/bridge/tc358768.c
@@ -9,6 +9,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/kernel.h>
+#include <linux/media-bus-format.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
@@ -918,6 +919,44 @@ static void tc358768_bridge_enable(struct drm_bridge *bridge)
 	}
 }
 
+#define MAX_INPUT_SEL_FORMATS	1
+
+static u32 *
+tc358768_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
+				   struct drm_bridge_state *bridge_state,
+				   struct drm_crtc_state *crtc_state,
+				   struct drm_connector_state *conn_state,
+				   u32 output_fmt,
+				   unsigned int *num_input_fmts)
+{
+	struct tc358768_priv *priv = bridge_to_tc358768(bridge);
+	u32 *input_fmts;
+
+	*num_input_fmts = 0;
+
+	input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
+			     GFP_KERNEL);
+	if (!input_fmts)
+		return NULL;
+
+	switch (priv->pd_lines) {
+	case 16:
+		input_fmts[0] = MEDIA_BUS_FMT_RGB565_1X16;
+		break;
+	case 18:
+		input_fmts[0] = MEDIA_BUS_FMT_RGB666_1X18;
+		break;
+	default:
+	case 24:
+		input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
+		break;
+	};
+
+	*num_input_fmts = MAX_INPUT_SEL_FORMATS;
+
+	return input_fmts;
+}
+
 static const struct drm_bridge_funcs tc358768_bridge_funcs = {
 	.attach = tc358768_bridge_attach,
 	.mode_valid = tc358768_bridge_mode_valid,
@@ -925,6 +964,11 @@ static const struct drm_bridge_funcs tc358768_bridge_funcs = {
 	.enable = tc358768_bridge_enable,
 	.disable = tc358768_bridge_disable,
 	.post_disable = tc358768_bridge_post_disable,
+
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_reset = drm_atomic_helper_bridge_reset,
+	.atomic_get_input_bus_fmts = tc358768_atomic_get_input_bus_fmts,
 };
 
 static const struct drm_bridge_timings default_tc358768_timings = {
-- 
2.25.1

Re: [PATCH v1 6/6] drm/bridge: tc358768: Add atomic_get_input_bus_fmts() implementation

[Francesco Dolcini]

Hello all,

On Thu, Mar 30, 2023 at 11:59:41AM +0200, Francesco Dolcini wrote:
> From: Francesco Dolcini <francesco.dolcini@toradex.com>
> 
> Add atomic_get_input_bus_fmts() implementation, tc358768 has a parallel
> RGB input interface with the actual bus format depending on the amount
> of parallel input data lines.
> 
> Without this change when the tc358768 is used with less than 24bit the
> color mapping is completely wrong.
> 
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>

Just a gently ping on this specific patch.

Other patches on this series received some comments that should be
addressed, however at the moment on that I am quite stuck.

On the other hand this patch is completely independent.

I do also have queued multiple fixes on the same driver that I
would need to send and would love to avoid conflicts between my own
patches.

How would you suggest to move forward? Can someone have a look at this
patch and if deemed fine can someone just pick it up?

Francesco