linux-kernel - Re: [PATCH 0/1] This patch fixes connection detection for monitors w/o DDC.

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: <CAGgjyvEA54kR3U8Lyz-1-vPS74raT6SpoM0e8YYcm12T=0r50A@mail.gmail.com>
Date:   Mon, 29 Jul 2019 10:58:06 +0000
From:   Oleksandr Suvorov <oleksandr.suvorov@...adex.com>
To:     "maxime.ripard@...e-electrons.com" <maxime.ripard@...e-electrons.com>
CC:     Andrzej Hajda <a.hajda@...sung.com>,
        Neil Armstrong <narmstrong@...libre.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        Igor Opaniuk <igor.opaniuk@...adex.com>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        Marcel Ziswiler <marcel.ziswiler@...adex.com>,
        Jonas Karlman <jonas@...boo.se>,
        David Airlie <airlied@...ux.ie>,
        Jernej Skrabec <jernej.skrabec@...l.net>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Suvorov Alexander <cryosay@...il.com>
Subject: Re: [PATCH 0/1] This patch fixes connection detection for monitors
 w/o DDC.

On Thu, Jul 25, 2019 at 5:41 PM maxime.ripard@...e-electrons.com
<maxime.ripard@...e-electrons.com> wrote:
>
> On Thu, Jul 25, 2019 at 11:05:23AM +0000, Oleksandr Suvorov wrote:
> >
> > Even in source code of this driver there is an author's description:
> >     /*
> >      * Even if we have an I2C bus, we can't assume that the cable
> >      * is disconnected if drm_probe_ddc fails. Some cables don't
> >      * wire the DDC pins, or the I2C bus might not be working at
> >      * all.
> >      */
> >
> > That's true. DDC and VGA channels are independent, and therefore
> > we cannot decide whether the monitor is connected or not,
> > depending on the information from the DDC.
> >
> > So the monitor should always be considered connected.
>
> Well, no. Like you said, we cannot decided whether is connected or
> not.

Maxime, thanks, I agree that's a bad solution.
But I still think we should be able to define the DT node of a device for
this driver to claim the connector is always connected.
Please see my following thoughts.

> > Thus there is no reason to use connector detect callback for this
> > driver: DRM sub-system considers monitor always connected if there
> > is no detect() callback registered with drm_connector_init().
> >
> > How to reproduce the bug:
> > * setup: i.MX8QXP, LCDIF video module + gpu/drm/mxsfb driver,
> >   adv712x VGA DAC + dumb-vga-dac driver, VGA-connector w/o DDC;
> > * try to use drivers chain mxsfb-drm + dumb-vga-dac;
> > * any DRM applications consider the monitor is not connected:
> >   ===========
> >   $ weston-start
> >   $ cat /var/log/weston.log
> >       ...
> >       DRM: head 'VGA-1' found, connector 32 is disconnected.
> >       ...
> >   $ cat /sys/devices/platform/5a180000.lcdif/drm/card0/card0-VGA-1/status
> >   unknown
>
> And that's exactly what's being reported here: we cannot decide if it
> is connected or not, so it's unknown.
>
> If weston chooses to ignore connectors that are in an unknown state,
> I'd say it's weston's problem, since it's much broader than this
> particular device.

If we look at the code of drm_probe_helper.c, we can see, the
drm_helper_probe_detect_ctx() assume the cable is connected if there is no
detect() callback registered.
...
                if (funcs->detect_ctx)
                         ret = funcs->detect_ctx(connector, &ctx, force);
                 else if (connector->funcs->detect)
                         ret = connector->funcs->detect(connector, force);
                 else
                         ret = connector_status_connected;
...

The driver dumb-vga-dac supports both DT configurations:
- with DDC channel, that allows us to detect if the cable is connected;
- without DDC channel. In this case, IMHO, the driver should behave
the same way as a
  connector driver without registered detect() callback.

So what about the patch like?

@@ -81,6 +81,13 @@ dumb_vga_connector_detect(struct drm_connector
*connector, bool force)
 {
        struct dumb_vga *vga = drm_connector_to_dumb_vga(connector);

+       /*
+        * If I2C bus for DDC is not defined, asume that the cable
+        * is always connected.
+        */
+       if (PTR_ERR(vga->ddc) == -ENODEV)
+               return connector_status_connected;
+
        /*
         * Even if we have an I2C bus, we can't assume that the cable
         * is disconnected if drm_probe_ddc fails. Some cables don't

-- 
Best regards
Oleksandr Suvorov

Toradex AG
Altsagenstrasse 5 | 6048 Horw/Luzern | Switzerland | T: +41 41 500
4800 (main line)