Re: [PATCH v2 0/3] drm: Add LVDS decoder bridge

[jacopo mondi]

Hi Andrzej,
    thanks for the detailed reply, much appreciated :)

On Mon, Mar 12, 2018 at 02:47:20PM +0100, Andrzej Hajda wrote:
> On 12.03.2018 13:30, jacopo mondi wrote:
> > Hi Andrzej,
> >
> > On Mon, Mar 12, 2018 at 10:07:27AM +0100, Andrzej Hajda wrote:
> >>

[snip]

> > I understand. The "transparent bridge" is of no actual use, but I don't see
> > how the "double bridge" thing is not an issue if I were to develop
> > support for the actual Thine chip.
>
> What is exactly your configuration: single/dual input, single/dual output?
> Current bindings suggests single/single, am I right? In such case you do
> not need to implement dual link functionality in the driver - since you
> even do not have possibility to test it.

Correct, I'm running in single/single mode.

> But the bindings should support all modes of operation, or at least
> should be easy to extend them in the future with backward compatibility.
>

And here is where I always get lost. For sake of being compatible with
future extensions, bindings shall describe hardware, not what is currently
supported in driver. One day I'll get this right!

> >
> > Please see my reply from yesterday to Archit. I still think having two
> > bridges is somehow an issue...
>
> Yes, I agree. But do we have such case? If no - no problem ATM :), if
> yes - lets try to implement it and see where is the problem, as Archit
> already suggested it would be just a matter of assigning bridge to port
> node, instead of device node.
>

Again, I've been fooled by the idea that if bindings describe
something, the driver should implement it (but I'm still not sure that
"just assign the port node to the bridge" is the right thing to do
here, but let's leave this out for now :)

> >
> > While we clarify that, would it be fine an initial driver version for
> > the actualt Thine chip with a single input support[1]? I would ditch this
> > transparent bridge and resume working on a THC63LVD1024 driver from
> > comments received on v1.
>
> I think, yes: driver with only single input, and full or extend-able
> bindings.
>
> >
> > Thanks
> >   j
> >
> > [1] Single input support implies a single input port in DT bindings
> > even if the chip supports two, and my understanding was that you
> > didn't like this.
>
> I am sorry if I was not clear enough, only thing I wanted was complete
> or at least consistent/extend-able binding.
> So when I saw word "multiple LVDS streams" in bindings I was looking
> further to see how these multiple LVDS bindings are defined, and found
> nothing :)

No worries, you're right here, again it's me confused by bindings
description vs driver features...

> Btw I think it may be better to use "Dual Link" instead of "Multiple
> streams", it is more precise and quite well established in docs.

Ack, I will follow up with a v3 where I'll get rid of generic LVDS
decoder and target the actual chip.

Thanks
   j
>
> Regards
> Andrzej
>
> >
> >
> >> Regards
> >> Andrzej
> >>
> >>> Jacopo Mondi (3):
> >>>   dt-bindings: display: bridge: Document LVDS to parallel decoder
> >>>   drm: bridge: Add LVDS decoder driver
> >>>   arm64: dts: renesas: Add LVDS decoder to R-Car V3M Eagle
> >>>
> >>>  .../bindings/display/bridge/lvds-decoder.txt       |  42 ++++++
> >>>  arch/arm64/boot/dts/renesas/r8a77970-eagle.dts     |  31 +++-
> >>>  drivers/gpu/drm/bridge/Kconfig                     |   8 ++
> >>>  drivers/gpu/drm/bridge/Makefile                    |   1 +
> >>>  drivers/gpu/drm/bridge/lvds-decoder.c              | 157 +++++++++++++++++++++
> >>>  5 files changed, 237 insertions(+), 2 deletions(-)
> >>>  create mode 100644 Documentation/devicetree/bindings/display/bridge/lvds-decoder.txt
> >>>  create mode 100644 drivers/gpu/drm/bridge/lvds-decoder.c
> >>>
> >>> --
> >>> 2.7.4
> >>>
> >>>
> >>>
> >>>
>

Attachment: signature.asc
Description: PGP signature