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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxxx>
Signed-off-by: Maxime Ripard <maxime.ripard@xxxxxxxxxxxxxxxxxx>
---
.../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".

Thanks,
Archit



Now that I've gotten that out of the way, this patch looks good to me
regardless of the name.

Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx>

Sean

In other words, this driver shouldn't be touched again in the future :)
If someone wants to write a RGB to VGA driver which is even
slightly configurable, they'll need to write a new bridge driver.

I'm sure that won't be true. I can certainly foresee the addition of
regulators support for instance. It's unfortunately never black and white.

What do you think about this? If you think it's good, would it be
possible for you to change this? I guess it's okay for the rest of
the patch to stay the same.

I'll update and respin the serie.


--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux