Re: [RFC PATCH 0/4] drm: bridge: anx7688 and mux drivers

[Archit Taneja <architt@codeaurora.org>]

On 07/06/2016 07:28 AM, Nicolas Boichat wrote:
> Hi Archit,
>
> Sorry got swamped by other things and forgot to reply.
>
> On Tue, Jun 28, 2016 at 4:28 PM, Archit Taneja <architt@codeaurora.org> wrote:
>>
>>
>> On 06/27/2016 01:18 PM, Nicolas Boichat wrote:
>>>
>>> Hi all,
>>>
>>> This is a follow up to the 2 patches to add support for ANX7688 sent here:
>>> https://patchwork.kernel.org/patch/9187809/, thanks Archit and Philipp for
>>> the comments.
>>>
>>> I also added 2 patches to add support for a simple display MUX, as I'm
>>> facing
>>> similar issues while trying to implement it, i.e. the current DRM core
>>> does not
>>> seem to support this kind of simple pass-thru bridge very well: it is not
>>> very
>>> clear where connectors should be defined and attached. In this case, not
>>> defining any connectors in the 2 bridges (and reusing the connector in MTK
>>> HDMI driver) seem to work best, but makes little logical sense as the
>>> physical
>>> connectors are actually attached to the bridges.
>>
>>
>> Bridges aren't really drm objects in themselves, they can just be
>> thought of as entities that attach to an encoder. From a drm
>> perspective, the connector is only linked to an encoder. It
>> doesn't see any bridges. Therefore, it doesn't matter much
>> if the bridge driver doesn't create connectors. The DT bindings,
>> however, should be close to the physical connections.
>>
>>>
>>> In any case, the board has the following layout:
>>>    - MT8173 HDMI bridge
>>>      - HDMI mux with 2 ports
>>>        1. ANX7688 for HDMI->DP over USB-C conversion
>>>        2. Native HDMI
>>>
>>
>> So, the MTK SoC's HDMI output (TMDS lines) can be routed to the
>> connector on the board directly (native mode), or via the ANX7688
>> bridge using the gpio mux. Did I get this part right?
>
> Yes.
>
>> Is there only one connector at the end of both the output paths?
>
> Err, there are:
>   - 2 physical connectors (HDMI, USB-C)
>   - 1 DT connector in the device tree (native HDMI), I don't bother
> adding a connector to anx7688, but in theory I could.
>   - 1 DRM connector object (defined in the mtk hdmi driver)
>
>>> The mux is controlled by hardware, looking at the HPD signals from both
>>> ANX7688
>>> and native HDMI, with a priority on the native HDMI output.
>>
>>
>> I didn't understand this. I can see that ANX7688 could generate a HPD
>> signal on behalf of the connected monitor, but why would the native MTK
>> HDMI controller generate a HPD signal? I would expect it to receive HPD
>> and trigger a CPU interrupt.
>>
>> Could you also give an idea about why the hardware switches between the
>> two paths? It it based on what kind of device plugs into the connector?
>
> The mux is controlled in hardware, by looking at both HPD lines:
>   - USB-C (HPD controlled by ANX7688, depending if there is a DP over
> USB-C device connected)
>   - native HDMI (HPD controlled by monitor on remote side)
>
> Note that, like the other HDMI lines, HPD is "muxed" (depending on the
> logic above), and wired up to MTK SoC HDMI/CEC module, which generates
> the interrupts.
>
> Priority is set to native HDMI port, so if both HPD signals are
> active, the output will stay on the HDMI port.

Thanks for the clarification.

It's a strange setup. It would be ideal to have 2 connectors visible to
userspace, with only one of them being 'DRM_MODE_CONNECTED' at a time.
There would be a bit of an overkill whenever userspace gets a hotplug
event, resulting in tearing down and setting up crtcs, encoders even
though nothing really changes much upstream.

I think the mux bridge part looks fine, but I don't know what's the best
way how to describe the connectors here :) (one drm_connector
representing both the physical connectors, or 2 separate drm_connector
entities which both can't be connected at the same time). Maybe someone
else from the list could help us out here.

Archit

>
> Best,
>
> Nicolas
>
>> Thanks,
>> Archit
>>
>>
>>>
>>> The whole setup works fairly well without any Linux kernel drivers, except
>>> the
>>> 2 following cases:
>>>    1. When ANX7688 is active, DP bandwidth may be limited, so we need to
>>> filter
>>>       resolutions that would exceed the available bandwidth.
>>>    2. When both outputs HPD signals are active, the kernel does not receive
>>> an
>>>       HPD pulse when the HDMI input is unplugged.
>>>
>>> ANX7688 driver fixes issue 1. The mux driver fixes 2 by forcing the kernel
>>> to
>>> re-read the EDID on mux output change, and also issue 1 by filtering only
>>> when
>>> ANX7688 is active.
>>>
>>> I understand this patch series might not be acceptable as-is, but I hope
>>> this
>>> sort of setup can be taken into account when better support for connector
>>> drivers is introduced.
>>>
>>> Thanks!
>>>
>>> Best,
>>>
>>> Nicolas
>>>
>>> Nicolas Boichat (4):
>>>     drm: bridge: anx7688: Add anx7688 bridge driver support.
>>>     devicetree: Add ANX7688 transmitter binding
>>>     drm: bridge: Generic GPIO mux driver
>>>     devicetree: Add GPIO display mux binding
>>>
>>>    .../devicetree/bindings/drm/bridge/anx7688.txt     |  32 ++
>>>    .../devicetree/bindings/drm/bridge/gpio-mux.txt    |  59 ++++
>>>    drivers/gpu/drm/bridge/Kconfig                     |  20 ++
>>>    drivers/gpu/drm/bridge/Makefile                    |   2 +
>>>    drivers/gpu/drm/bridge/analogix-anx7688.c          | 233 ++++++++++++++
>>>    drivers/gpu/drm/bridge/generic-gpio-mux.c          | 347
>>> +++++++++++++++++++++
>>>    6 files changed, 693 insertions(+)
>>>    create mode 100644
>>> Documentation/devicetree/bindings/drm/bridge/anx7688.txt
>>>    create mode 100644
>>> Documentation/devicetree/bindings/drm/bridge/gpio-mux.txt
>>>    create mode 100644 drivers/gpu/drm/bridge/analogix-anx7688.c
>>>    create mode 100644 drivers/gpu/drm/bridge/generic-gpio-mux.c
>>>
>>
>> --
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> a Linux Foundation Collaborative Project

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project