linux-kernel - Re: [PATCH] drm/panel: add CONFIG_DRM_KMS

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: <CAK8P3a3ogV8sJshQJrzHe7hc2k1Opq-2hvXf6DU+HfhREk-DhQ@mail.gmail.com>
Date:   Wed, 16 Mar 2022 21:59:38 +0100
From:   Arnd Bergmann <arnd@...nel.org>
To:     Thomas Zimmermann <tzimmermann@...e.de>
Cc:     Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Thierry Reding <thierry.reding@...il.com>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>,
        Kees Cook <keescook@...omium.org>,
        Arnd Bergmann <arnd@...db.de>,
        Jani Nikula <jani.nikula@...el.com>,
        Sam Ravnborg <sam@...nborg.org>,
        Javier Martinez Canillas <javierm@...hat.com>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Douglas Anderson <dianders@...omium.org>,
        Deepak Rawat <drawat.floss@...il.com>,
        Noralf Trønnes <noralf@...nnes.org>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...ainline.org>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Alex Deucher <alexander.deucher@....com>,
        Dillon Min <dillon.minfei@...il.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/panel: add CONFIG_DRM_KMS_HELPER dependencies

On Wed, Mar 16, 2022 at 8:31 PM Thomas Zimmermann <tzimmermann@...e.de> wrote:
> Am 16.03.22 um 20:12 schrieb Thomas Zimmermann:
> >>
> >> Adding a dependency in all drivers that select DRM_MIPI_DBI avoids
> >> the problem for now, adding the dependency in DRM_MIPI_DBI as well
> >> should help make it easier to figure out why it breaks if someone
> >> forgets the dependency the next time.
> >>
> >>       tristate
> >> -    depends on DRM
> >> +    depends on DRM_KMS_HELPER
> >
> > This symbol cannot be selected by users, so it's maybe not a good idea
> > to depend on it. In fact, I've had to remove such a statement because it
> > created a cyclic dependency. [1]

I tried to explain above what I was thinking here: the added dependency
is both a correct statement (DRM_MIPI_DBI depends on DRM_KMS_HELPER
because it cannot be built without DRM_KMS_HELPER) and helpful as
an indication what went wrong if we run into the same problem with a new
driver, instead of the cryptic link failure you get something like

WARNING: unmet direct dependencies detected for DRM_MIPI_DBI
  Depends on [m]: HAS_IOMEM [=y] && DRM_KMS_HELPER [=m]
  Selected by [y]:
  - DRM_PANEL_WIDECHIPS_WS2401 [=y] && HAS_IOMEM [=y] && DRM [=y] &&
DRM_PANEL [=y] && SPI [=y] && GPIOLIB [=y] && BACKLIGHT_CLASS_DEVICE
[=y]
  Selected by [m]:
  - TINYDRM_ILI9225 [=m] && HAS_IOMEM [=y] && DRM [=y] && SPI [=y]

> [1]
> https://lore.kernel.org/dri-devel/20220315084559.23510-1-tzimmermann@suse.de/

I was going for 'depends on' in the panel drivers because I saw the same being
done for other panel drivers, and mixing the two methods causes dependency
loops. I looked again now, and find that 'select DRM_KMS_HELPER' is more
common for other drivers, and makes sense here because it is generally
not user-selectable.

The easiest replacement for my patch would then be to just use 'select
DRM_KMS_HELPER' from CONFIG_DRM_MIPI_DBI, which makes it
safer and more consistent with your change. If you like, I'll send an updated
version.

One thing I'm not sure about is whether there is still use for ever having
CONFIG_DRM without CONFIG_DRM_KMS_HELPER if it gets selected
by almost every driver anyway. Is this actually a configuration that
users rely on, or should we just remove the symbol completely and
build the KMS helpers unconditionally?

       Arnd