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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Archit,

On Friday 07 Oct 2016 10:27:31 Archit Taneja wrote:
> On 10/07/2016 02:34 AM, Laurent Pinchart wrote:
> > 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.t
> >>>>>>> xt
> >>>>>>> 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,

I'm testing the patch with rcar-du-drm and will provide results today.

> and change the compatible to "dumb-vga-dac".

Feel free to ignore the bikeshedding, but "transparent" could be a candidate 
to replace "dumb" (either as "vga-dac-transparent" or "transparent-vga-dac").

-- 
Regards,

Laurent Pinchart

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