Re: [PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Maxime Ripard <maxime.ripard@free-electrons.com>]

[-- Attachment #1.1: Type: text/plain, Size: 5417 bytes --]

On Fri, Oct 07, 2016 at 10:27:31AM +0530, Archit Taneja wrote:
> 
> 
> On 10/07/2016 02:34 AM, Laurent Pinchart wrote:
> >Hi Sean,
> >
> >On Thursday 06 Oct 2016 15:53:28 Sean Paul wrote:
> >>On Thu, Oct 6, 2016 at 1:27 PM, Laurent Pinchart wrote:
> >>>On Thursday 06 Oct 2016 17:09:57 Archit Taneja wrote:
> >>>>On 10/06/2016 12:51 PM, Maxime Ripard wrote:
> >>>>>On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
> >>>>>>On 09/30/2016 08:07 PM, Maxime Ripard wrote:
> >>>>>>>Some boards have an entirely passive RGB to VGA bridge, based on
> >>>>>>>either DACs or resistor ladders.
> >>>>>>>
> >>>>>>>Those might or might not have an i2c bus routed to the VGA connector
> >>>>>>>in order to access the screen EDIDs.
> >>>>>>>
> >>>>>>>Add a bridge that doesn't do anything but expose the modes available
> >>>>>>>on the screen, either based on the EDIDs if available, or based on
> >>>>>>>the XGA standards.
> >>>>>>>
> >>>>>>>Acked-by: Rob Herring <robh@kernel.org>
> >>>>>>>Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >>>>>>>---
> >>>>>>>.../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
> >>>>>>>drivers/gpu/drm/bridge/Kconfig                     |   7 +
> >>>>>>>drivers/gpu/drm/bridge/Makefile                    |   1 +
> >>>>>>>drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++
> >>>>>>>4 files changed, 285 insertions(+)
> >>>>>>>create mode 100644
> >>>>>>>Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
> >>>>>>>t
> >>>>>>>create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
> >>>>>>>
> >>>>>>>diff --git
> >>>>>>>a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
> >>>>>>>txt
> >>>>>>>b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
> >>>>>>>txt
> >>>>>>>new file mode 100644
> >>>>>>>index 000000000000..a8375bc1f9cb
> >>>>>>>--- /dev/null
> >>>>>>>+++
> >>>>>>>b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
> >>>>>>>tx
> >>>>>>>t @@ -0,0 +1,48 @@
> >>>>>>>+Dumb RGB to VGA bridge
> >>>>>>>+----------------------
> >>>>>>>+
> >>>>>>>+This binding is aimed for dumb RGB to VGA bridges that do not
> >>>>>>>require
> >>>>>>>+any configuration.
> >>>>>>>+
> >>>>>>>+Required properties:
> >>>>>>>+
> >>>>>>>+- compatible: Must be "rgb-to-vga-bridge"
> >>>>>>
> >>>>>>I'd talked to Laurent on IRC if he's okay with this. And I guess you
> >>>>>>to had discussed it during XDC too. He's suggested that it'd be better
> >>>>>>to have the compatible string as "simple-vga-dac".
> >>>>>
> >>>>>I just wished this bikeshedding had taken place publicly and be
> >>>>>actually part of that discussion, but yeah, ok.
> >>>>
> >>>>Sorry about that. I'd pinged him for an Ack, the discussion went
> >>>>more than that :)
> >>>>
> >>>>>>Some of the reasons behind having this:
> >>>>>>
> >>>>>>- We don't need to specify "rgb" in the compatible string since most
> >>>>>>simple VGA DACs can only work with an RGB input.
> >>>>>
> >>>>>Ok.
> >>>>>
> >>>>>>- Also, with "dac" specified in the string, we don't need to
> >>>>>>specifically mention "bridge" in the string. Also, bridge is a drm
> >>>>>>specific term.
> >>>>>>
> >>>>>>- "simple" is considered because it's an unconfigurable bridge, and it
> >>>>>>might be misleading for other VGA DACs to not use "vga-dac".
> >>>>>
> >>>>>All those "simple" bindings are just the biggest lie we ever
> >>>>>told. It's simple when you introduce it, and then grows into something
> >>>>>much more complicated than a non-simple implementation.
> >>>>
> >>>>"simple" here is supposed to mean that it's an unconfigurable RGB to
> >>>>VGA DAC. This isn't supposed to follow the simple-panel model, where
> >>>>you add the "simple-panel" string in the compatible node, along with
> >>>>you chip specific compatible string.
> >>>
> >>>I agree with Maxime, I don't like the word "simple". My preference would
> >>>be "vga-dac" for a lack of a better qualifier than "simple" to describe
> >>>the fact that the device requires no configuration. My only concern with
> >>>"vga-dac" is that we would restrict usage of that compatible string for a
> >>>subset of VGA DACs, with more complex devices not being compatible with
> >>>"vga-dac" even though they are VGA DACs. That's a problem I can live with
> >>>though.
> >>
> >>While we're bikeshedding (feel free to ignore my input on this), I
> >>think Maxime's initial "dumb" qualifier was better than "simple".
> >
> >I think I agree.
> >
> >>I think "passive" also gets the point across better than "simple", which
> >>we've already established as something else in drm.
> >
> >To my electrical engineer's ear, passive refers to a component or combination
> >of components that is not capable of power gain. The resistors ladder VGA DAC
> >that Maxime is trying to support is a passive system, but the ADV7123 VGA DAC
> >that equally requires no configuration is an active component.
> 
> If no one has any more objections within the next day, I'll pull in
> Maxime's v5 RGB to VGA bridge driver, and change the compatible to
> "dumb-vga-dac".

That works for me. You'll probably want to update the Kconfig and file
name to match though.

Maxime

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

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel