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. > 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. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
