All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>, Kees Cook <keescook@chromium.org>
Cc: "Arnd Bergmann" <arnd@kernel.org>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"David Airlie" <airlied@linux.ie>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	"Javier Martinez Canillas" <javierm@redhat.com>
Subject: Re: [PATCH] [RESEND] drm: fb_helper: fix CONFIG_FB dependency
Date: Wed, 27 Oct 2021 14:47:37 +0300	[thread overview]
Message-ID: <878ryeit9i.fsf@intel.com> (raw)
In-Reply-To: <YVXJLE8UqgcUNIKl@phenom.ffwll.local>

On Thu, 30 Sep 2021, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Sep 27, 2021 at 09:23:45AM -0700, Kees Cook wrote:
>> On Mon, Sep 27, 2021 at 04:28:02PM +0200, Arnd Bergmann wrote:
>> > From: Arnd Bergmann <arnd@arndb.de>
>> > 
>> > With CONFIG_FB=m and CONFIG_DRM=y, we get a link error in the fb helper:
>> > 
>> > aarch64-linux-ld: drivers/gpu/drm/drm_fb_helper.o: in function `drm_fb_helper_alloc_fbi':
>> > (.text+0x10cc): undefined reference to `framebuffer_alloc'
>> > 
>> > Tighten the dependency so it is only allowed in the case that DRM can
>> > link against FB.
>> > 
>> > Fixes: f611b1e7624c ("drm: Avoid circular dependencies for CONFIG_FB")
>> > Link: https://lore.kernel.org/all/20210721152211.2706171-1-arnd@kernel.org/
>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> 
>> Thanks for fixing this!
>> 
>> Reviewed-by: Kees Cook <keescook@chromium.org>
>
> Stuffed into drm-misc-next.

The problem is, I don't think the patch is semantically correct.

drm_fb_helper.o is not part of drm.ko, it's part of
drm_kms_helper.ko. This adds some sort of indirect dependency via DRM
which might work, maybe by coincidence, maybe not - but it's certainly
not obvious.

The likely culprit is, again, the overuse of select, and in this case
select DRM_KMS_HELPER. And DRM_KMS_HELPER should depend on FB if
DRM_FBDEV_EMULATION=y. That's the problem.

All of the drm Kconfigs could use an overhaul to be semantically
correct, but that's a hill nobody wants to die on. Instead we keep
piling up tweaks to paper over the issues, ad infinitum.

(And this ties to a previous comment I had about the organization of
files under drm/, a hundred files in one big lump that belong to
different modules, and it's not helping people figure out the
dependencies.)


BR,
Jani.


PS. I was brought here via [1] which is another complicated "fix" to the
same problem.


[1] https://lore.kernel.org/r/20211027072044.4105113-1-javierm@redhat.com


-- 
Jani Nikula, Intel Open Source Graphics Center

  reply	other threads:[~2021-10-27 11:47 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-27 14:28 [PATCH] [RESEND] drm: fb_helper: fix CONFIG_FB dependency Arnd Bergmann
2021-09-27 16:23 ` Kees Cook
2021-09-30 14:26   ` Daniel Vetter
2021-10-27 11:47     ` Jani Nikula [this message]
2021-10-27 12:13       ` Jani Nikula
2021-10-27 12:18       ` Arnd Bergmann
2021-10-27 12:38         ` Javier Martinez Canillas
2021-10-27 12:52           ` Arnd Bergmann
2021-10-27 12:56             ` Javier Martinez Canillas
2021-10-27 13:06             ` Jani Nikula
2021-10-27 13:25               ` Arnd Bergmann
2021-10-27 13:36                 ` Javier Martinez Canillas
2021-10-27 12:55           ` Jani Nikula
2021-10-27 13:18             ` Javier Martinez Canillas
2021-10-27 13:05         ` Jani Nikula
2021-10-27 13:19         ` Javier Martinez Canillas
2021-10-28 15:24           ` Daniel Vetter
2021-10-29 12:06             ` Arnd Bergmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878ryeit9i.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=airlied@linux.ie \
    --cc=alexander.deucher@amd.com \
    --cc=arnd@arndb.de \
    --cc=arnd@kernel.org \
    --cc=christian.koenig@amd.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=javierm@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.