linux-kernel - Re: [RFC PATCH 1/2] drm: bridge: anx7688: Add anx7688 bridge driver support.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANMq1KBkU0FtABqnqLbJwtQTrh1Vgxe69Dyu4cznJmyGExzBzg@mail.gmail.com>
Date:	Thu, 23 Jun 2016 12:08:11 +0800
From:	Nicolas Boichat <drinkcat@...omium.org>
To:	Philipp Zabel <p.zabel@...gutronix.de>
Cc:	Archit Taneja <architt@...eaurora.org>,
	dri-devel@...ts.freedesktop.org,
	Stéphane Marchesin <marcheu@...omium.org>,
	linux-kernel@...r.kernel.org,
	Enric Balletbo i Serra <enric.balletbo@...labora.com>,
	Russell King <rmk+kernel@....linux.org.uk>,
	Thierry Reding <treding@...dia.com>
Subject: Re: [RFC PATCH 1/2] drm: bridge: anx7688: Add anx7688 bridge driver support.

Hi Philipp,

On Wed, Jun 22, 2016 at 4:51 PM, Philipp Zabel <p.zabel@...gutronix.de> wrote:
> Hi Nicolas,
>
> Am Mittwoch, den 22.06.2016, 14:32 +0800 schrieb Nicolas Boichat:
>> >> Actually, experimenting a bit more with the code, I realized that the
>> >> connector is always attached to the encoder, not the bridge, so the 2
>> >> layouts above are actually identical (from the userspace point of
>> >> view). Except that the connector name should be HDMI in one case, and
>> >> DP in the other. But I think that's mostly a cosmetic difference?
>> >
>> >
>> > Yeah, probably. I don't know what exactly the userspace does with
>> > the connector type, but it would be nice to represent it as a DP
>> > connector in case it makes any decisions based on it.
>>
>> AFAICT does not matter... And, in any case, USB-C to HDMI adapters are
>> far more common (so we'd do HDMI->(DP over USB-C)->HDMI....), so there
>> is a high change that advertising as DP would be wrong (and
>> advertising as HDMI would be correct by chance...).
>
> If anything it should be represented as an USB-C connector, whatever
> USB-C(DP)->HDMI->VGA adapter chain is connected externally shouldn't
> matter.

Indeed, looks like we could just create a new connector type.

>> >>> One way I can think of fixing this is to make make the MTK hdmi
>> >>> encoder driver a bit smarter by observing the DT connections. If
>> >>> it's output port is connected to just a hdmi-connector, then
>> >>> things should be as before. If the output is connected to the DP
>> >>> bridge, then it should create a DP connector. The connector ops
>> >>> for the DP connector can still be the same as that of the HDMI
>> >>> connector before, but the phandle to the DDC bus would be in the
>> >>> DP device node in this case.
>> >>
>> >>
>> >> I think it'd be a bit weird to have the DDC bus phandle on the DP
>> >> connector, as we're not reading the EDID on the DP side of the bridge,
>> >> but on the HDMI side (and the bridge can do all sort of things to the
>> >> EDID: At the very least, I think it caches it).
>>>
>> > On the board, do the DDC lines join directly from the SoC pins to the
>> > DP connector, or does it go via the ANX7688 chip?
>>
>> The DDC/I2C lines go to the ANX7688 chip. (DP has AUX channel instead)
>
> The anx7688 should get the i2c-ddc-bus property then, and the driver
> should use it to implement get_modes (or its bridge API equivalent, once
> it exists).

Ok, I can do that (I tried it). It's a lot a duplication from the
existing connector in mtk-hdmi, but I understand there may not be a
better way currently.

However, the problem I'm facing is how to implement "detect" in the
connector function. I'm still getting the interrupts from the MTK cec
module (so the detect function gets called), but then I don't see a
good way to read the HPD status (ANX7688 does not have a register for
that: it passes the HPD signal downstream).

mtk_hdmi implements detect this way:
        return mtk_cec_hpd_high(hdmi->cec_dev) ?
               connector_status_connected : connector_status_disconnected;

But of course I can't call this function from the ANX7688 bridge driver...

>> > Even with the current bindings (referred from the link below), the
>> > hdmi connector has the DDC node. Shouldn't it be the same in the DP
>> > case too? The DP connector, like before, is still manged by the HDMI
>> > driver, the only difference being the name and that it's two hops away
>> > in the DT bindings.
>> >
>> > https://patchwork.kernel.org/patch/9137089/
>>
>> True, the bindings say so, but the current code actually looks for
>> ddc-i2c-bus property in whatever is connected on the endpoint (be it a
>> bridge or a connector).
>
> The HDMI driver only handles this itself because there are no separate
> connector drivers (or helpers) to do this. Ideally the HDMI driver
> shouldn't have to parse the connector DT node.
>
>> And again, a bit odd as DP connector does not have true I2C lines...
>>
>> Phillip? Any opinion?
>
> Make the HDMI driver leave the bridges' DT node alone and let the bridge
> handle DDC, if that's where it is connected physically.

Again, a bit odd... AFAICT, all other bridge drivers, and encoders,
read EDID from a bus that is downstream of it. If we handle DDC in the
ANX driver, it'll have to read the EDID upstream (on the HDMI side),
as there is no way to read the EDID on the DP side on this chip.

> regards
> Philipp
>

Thanks!

Best,