Received-SPF: pass (google.com: domain of linux-kernel+bounces-153333-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.11.0-alpha0-379-gabd37849b7-fm-20240408.001-gabd37849
Precedence: bulk
MIME-Version: 1.0
Message-Id: <ff4f9e8f-0825-4421-adf9-e3914b108da7@app.fastmail.com>
In-Reply-To: <87il09ty4u.fsf@intel.com>
References: <cover.1713780345.git.geert+renesas@glider.be>
 <87il09ty4u.fsf@intel.com>
Date: Mon, 22 Apr 2024 14:30:18 +0200
From: "Arnd Bergmann" <arnd@arndb.de>
To: "Jani Nikula" <jani.nikula@linux.intel.com>,
 "Geert Uytterhoeven" <geert+renesas@glider.be>,
 "Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
 "Maxime Ripard" <mripard@kernel.org>,
 "Thomas Zimmermann" <tzimmermann@suse.de>, "Dave Airlie" <airlied@gmail.com>,
 "Daniel Vetter" <daniel@ffwll.ch>
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
 Linux-Renesas <linux-renesas-soc@vger.kernel.org>
Subject: Re: [PATCH 00/11] drm: Restore helper usability
Content-Type: text/plain

On Mon, Apr 22, 2024, at 13:50, Jani Nikula wrote:
> On Mon, 22 Apr 2024, Geert Uytterhoeven <geert+renesas@glider.be> wrote:
>> 	Hi all,
>>
>> As discussed on IRC with Maxime and Arnd, this series reverts the
>> conversion of select to depends for various DRM helpers in series
>> "[PATCH v3 00/13] drm/display: Convert helpers Kconfig symbols to
>> depends on"[1], and various fixes for it.  This conversion introduced a
>> big usability issue when configuring a kernel and enabling DRM drivers
>> that use DRM helper code: as drivers now depend on helpers, the user
>> needs to know which helpers to enable, before the driver he is
>> interested even becomes visible.  The user should not need to know that,
>> and drivers should select the helpers they need.
>>
>> Hence revert back to what we had before, where drivers selected the
>> helpers (and any of their dependencies, if they can be met) they need.
>> In general, when a symbol selects another symbol, it should just make
>> sure the dependencies of the target symbol are met, which may mean
>> adding dependencies to the source symbol.

Thanks for doing this.

Acked-by: Arnd Bergmann <arnd@arndb.de>

> I still disagree with this, because fundamentally the source symbol
> really should not have to care about the dependencies of the target
> symbol.

Sorry you missed the IRC discussion on #armlinux, we should have
included you as well since you applied the original patch.

I think the reason for this revert is a bit more nuanced than
just the usability problem. Sorry if I'm dragging this out too
much, but I want to be sure that two points come across:

1. There is a semantic problem that is mostly subjective, but
   with the naming as "helper", I generally expect it as a hidden
   symbol that gets selected by its users, while calling same module
   "feature" would be something that is user-enabled and that
   other modules depend on. Both ways are commonly used in the
   kernel and are not mistakes on their own.

2. Using "select" on user visible symbols that have dependencies
   is a common source for bugs, and this is is a problem in
   drivers/gpu/drm more than elsewhere in the kernel, as these
   drivers traditionally select entire subsystems or drivers
   (I2C, VIRTIO, INPUT, ACPI_WMI, BACKLIGHT_CLASS_DEVICE,
   POWER_SUPPLY, SND_PCM, INTERCONNECT, ...). This regularly
   leads to circular dependencies and we should fix all of them.
   The display helpers however don't have this problem because
   they do not have any dependencies outside of drivers/gpu/

        Arnd