Message ID | 20230629121952.10559-8-tzimmermann@suse.de |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp9599296vqr; Thu, 29 Jun 2023 05:27:49 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6E9eijhlC7mDqxaiBIJmucD6IVCnT7JZGLZZM5oeAlfBAz3uFfhA/erKzB7HN5iTVMwgsJ X-Received: by 2002:a05:6a21:9985:b0:127:2dc1:c885 with SMTP id ve5-20020a056a21998500b001272dc1c885mr15622318pzb.4.1688041668623; Thu, 29 Jun 2023 05:27:48 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1688041668; cv=none; d=google.com; s=arc-20160816; b=Dx6X4X57UxxEE4L7NEMKPrKZ4UNIbIhfuPrb2xr1S+hKR/XaBo49S1iyJOAT0QCIEJ vTQdhWW7BWiGS3DX/npj0ZbjND5cP9ZU453kfrzs5zN9Cq2fsOx9Apn+zT6ePYgwdAIl N57+A78tH4H5c+ua7OFpLKmBOsOLTzHNuM0+ciGNztdWJ1rnpu9S9MMMfDKy5bwtPHIy mmw3GJbLwTA7Y4HJtVBpOwPmVpQPRsF2rzggj1ezL8V2zYo0g2SlVWKL7rnR6HFwH4y0 oPdt4Uo0ZfCdFgrQXl12/TL0UmofeOtj/26eHeSXK3BxV5tnbLw22rezhlsYftEc1kox N8GQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature:dkim-signature; bh=cQ3EDeCSnwNHCvqI9o+8mLXg+vNxeu9dqjNgbn6vW64=; fh=QPiqliwnRA2MKgqFWQvZMiCC1n9E3Iht+6o2rdrCqhk=; b=07lafIrf1iYzHlxteh5Zf8StEcLVPrzBsjnmMyM7HVwewarciyErCW9X0yI6Do16h3 sFx3cHb8NVfbAGhq3Mw5CLEvzdv02HQiQJ4x0RppaZv01o+NNOl3DoAPZG4kf7lY7Zta vWGe5FwUPWWuhxSoaTvLmwyN6IyCBhqI8CDeWxh8m7xkp69d4sGTa8IZGFEyaTCrpK0X X5Jni+D3o62isMbiiRH9pcBY6BENR3xBeVNVbOLItugFaY9XoLkZONvcpP+mEuiDfhFU FV04KHbyVfJYvpkuMzvSrSb9tr2Rcbc1MkUqfaZbZJF404Rnl6It5Iad+8TLv6PXhQCH pcSg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@suse.de header.s=susede2_rsa header.b="d/ONtzMH"; dkim=neutral (no key) header.i=@suse.de header.s=susede2_ed25519 header.b=f3OsHAg+; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=suse.de Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id c37-20020a631c65000000b00544c0bf5ffesi10428457pgm.606.2023.06.29.05.27.35; Thu, 29 Jun 2023 05:27:48 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@suse.de header.s=susede2_rsa header.b="d/ONtzMH"; dkim=neutral (no key) header.i=@suse.de header.s=susede2_ed25519 header.b=f3OsHAg+; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=suse.de Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232772AbjF2MVO (ORCPT <rfc822;ivan.orlov0322@gmail.com> + 99 others); Thu, 29 Jun 2023 08:21:14 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52304 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232236AbjF2MUD (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 29 Jun 2023 08:20:03 -0400 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A7BD5358A; Thu, 29 Jun 2023 05:20:01 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 1C0E72186B; Thu, 29 Jun 2023 12:20:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1688041200; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=cQ3EDeCSnwNHCvqI9o+8mLXg+vNxeu9dqjNgbn6vW64=; b=d/ONtzMHBHfhPFGJZXFbwxLHKekEj3Ck140ZBA9jozx6NZ9IQncD0ogM4FlYnbVbwD45G1 SJ8ei5BmHWMBBnBkm9jXuvlD6yHYAuy3e3PYj4Jw61gdgEKEw5UzbK+mo0r3LVmeUysWir EVI14bZZuCFygATwSv7zxT0YU4yItG4= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1688041200; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=cQ3EDeCSnwNHCvqI9o+8mLXg+vNxeu9dqjNgbn6vW64=; b=f3OsHAg+MvgJYkBzRW/rb3tMUgPoqd73LlNMm0j7gkGvTcrmNDsm3dmRC0NYT1egdnRYUJ PNHfPSmv46ciy1AA== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 61E3213A43; Thu, 29 Jun 2023 12:19:59 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id wFf0Fu92nWRlVAAAMHmgww (envelope-from <tzimmermann@suse.de>); Thu, 29 Jun 2023 12:19:59 +0000 From: Thomas Zimmermann <tzimmermann@suse.de> To: arnd@arndb.de, deller@gmx.de, daniel@ffwll.ch, airlied@gmail.com Cc: linux-kernel@vger.kernel.org, linux-alpha@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-efi@vger.kernel.org, linux-csky@vger.kernel.org, linux-hexagon@vger.kernel.org, linux-ia64@vger.kernel.org, loongarch@lists.linux.dev, linux-mips@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-riscv@lists.infradead.org, linux-sh@vger.kernel.org, sparclinux@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-hyperv@vger.kernel.org, linux-fbdev@vger.kernel.org, linux-staging@lists.linux.dev, linux-arch@vger.kernel.org, Thomas Zimmermann <tzimmermann@suse.de>, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>, Kees Cook <keescook@chromium.org>, "Paul E. McKenney" <paulmck@kernel.org>, Peter Zijlstra <peterz@infradead.org>, Andrew Morton <akpm@linux-foundation.org>, Frederic Weisbecker <frederic@kernel.org>, Nicholas Piggin <npiggin@gmail.com>, Ard Biesheuvel <ardb@kernel.org>, Sami Tolvanen <samitolvanen@google.com>, Juerg Haefliger <juerg.haefliger@canonical.com> Subject: [PATCH 07/12] arch/x86: Declare edid_info in <asm/screen_info.h> Date: Thu, 29 Jun 2023 13:45:46 +0200 Message-ID: <20230629121952.10559-8-tzimmermann@suse.de> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230629121952.10559-1-tzimmermann@suse.de> References: <20230629121952.10559-1-tzimmermann@suse.de> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1770039980664282223?= X-GMAIL-MSGID: =?utf-8?q?1770039980664282223?= |
Series |
arch,fbdev: Move screen_info into arch/
|
|
Commit Message
Thomas Zimmermann
June 29, 2023, 11:45 a.m. UTC
The global variable edid_info contains the firmware's EDID information
as an extension to the regular screen_info on x86. Therefore move it to
<asm/screen_info.h>.
Add the Kconfig token ARCH_HAS_EDID_INFO to guard against access on
architectures that don't provide edid_info. Select it on x86.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: x86@kernel.org
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Helge Deller <deller@gmx.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Kees Cook <keescook@chromium.org>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Frederic Weisbecker <frederic@kernel.org>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: Juerg Haefliger <juerg.haefliger@canonical.com>
---
arch/Kconfig | 6 ++++++
arch/x86/Kconfig | 1 +
drivers/video/Kconfig | 3 ---
include/asm-generic/screen_info.h | 6 ++++++
include/video/edid.h | 3 ---
5 files changed, 13 insertions(+), 6 deletions(-)
Comments
On Thu, Jun 29, 2023, at 13:45, Thomas Zimmermann wrote: > The global variable edid_info contains the firmware's EDID information > as an extension to the regular screen_info on x86. Therefore move it to > <asm/screen_info.h>. > > Add the Kconfig token ARCH_HAS_EDID_INFO to guard against access on > architectures that don't provide edid_info. Select it on x86. I'm not sure we need another symbol in addition to CONFIG_FIRMWARE_EDID. Since all the code behind that existing symbol is also x86 specific, would it be enough to just add either 'depends on X86' or 'depends on X86 || COMPILE_TEST' there? Arnd
Hi Am 29.06.23 um 14:35 schrieb Arnd Bergmann: > On Thu, Jun 29, 2023, at 13:45, Thomas Zimmermann wrote: >> The global variable edid_info contains the firmware's EDID information >> as an extension to the regular screen_info on x86. Therefore move it to >> <asm/screen_info.h>. >> >> Add the Kconfig token ARCH_HAS_EDID_INFO to guard against access on >> architectures that don't provide edid_info. Select it on x86. > > I'm not sure we need another symbol in addition to > CONFIG_FIRMWARE_EDID. Since all the code behind that > existing symbol is also x86 specific, would it be enough > to just add either 'depends on X86' or 'depends on X86 || > COMPILE_TEST' there? FIRMWARE_EDID is a user-selectable feature, while ARCH_HAS_EDID_INFO announces an architecture feature. They do different things. Right now, ARCH_HAS_EDID_INFO only works on the old BIOS-based VESA systems. In the future, I want to add support for EDID data from EFI and OF as well. It would be stored in edid_info. I assume that the new symbol will become useful then. Before this patchset, I originally wanted to make use of edid_info in the simpledrm graphics driver. But I found that the rsp code could use some work first. Maybe the patchset are already tailored towards the future changes. Best regards Thomas > > Arnd -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
On Thu, Jun 29, 2023, at 15:01, Thomas Zimmermann wrote: > Am 29.06.23 um 14:35 schrieb Arnd Bergmann: >> On Thu, Jun 29, 2023, at 13:45, Thomas Zimmermann wrote: >>> The global variable edid_info contains the firmware's EDID information >>> as an extension to the regular screen_info on x86. Therefore move it to >>> <asm/screen_info.h>. >>> >>> Add the Kconfig token ARCH_HAS_EDID_INFO to guard against access on >>> architectures that don't provide edid_info. Select it on x86. >> >> I'm not sure we need another symbol in addition to >> CONFIG_FIRMWARE_EDID. Since all the code behind that >> existing symbol is also x86 specific, would it be enough >> to just add either 'depends on X86' or 'depends on X86 || >> COMPILE_TEST' there? > > FIRMWARE_EDID is a user-selectable feature, while ARCH_HAS_EDID_INFO > announces an architecture feature. They do different things. I still have trouble seeing the difference. > Right now, ARCH_HAS_EDID_INFO only works on the old BIOS-based VESA > systems. In the future, I want to add support for EDID data from EFI and > OF as well. It would be stored in edid_info. I assume that the new > symbol will become useful then. I don't see why an OF based system would have the same limitation as legacy BIOS with supporting only a single monitor, if we need to have a generic representation of EDID data in DT, that would probably be in a per device property anyway. I suppose you could use FIRMWARE_EDID on EFI or OF systems without the need for a global edid_info structure, but that would not share any code with the current fb_firmware_edid() function. Arnd
Hi Am 29.06.23 um 15:21 schrieb Arnd Bergmann: > On Thu, Jun 29, 2023, at 15:01, Thomas Zimmermann wrote: >> Am 29.06.23 um 14:35 schrieb Arnd Bergmann: >>> On Thu, Jun 29, 2023, at 13:45, Thomas Zimmermann wrote: >>>> The global variable edid_info contains the firmware's EDID information >>>> as an extension to the regular screen_info on x86. Therefore move it to >>>> <asm/screen_info.h>. >>>> >>>> Add the Kconfig token ARCH_HAS_EDID_INFO to guard against access on >>>> architectures that don't provide edid_info. Select it on x86. >>> >>> I'm not sure we need another symbol in addition to >>> CONFIG_FIRMWARE_EDID. Since all the code behind that >>> existing symbol is also x86 specific, would it be enough >>> to just add either 'depends on X86' or 'depends on X86 || >>> COMPILE_TEST' there? >> >> FIRMWARE_EDID is a user-selectable feature, while ARCH_HAS_EDID_INFO >> announces an architecture feature. They do different things. > > I still have trouble seeing the difference. The idea here is that ARCH_HAS_ signals the architecture's support for the feature. Drivers set 'depends on' in their Kconfig. Another Kconfig token, VIDEO_SCREEN_INFO or FIRMWARE_EDID, would then actually enable the feature. Drivers select VIDEO_SCREEN_INFO or FIRMWARE_EDID and the architectures contains code like #ifdef VIDEO_SCREEN_INFO struct screen_info screen_info = { /* set values here */ } #endif This allows us to disable code that requires screen_info/edid_info, but also disable screen_info/edid_info unless such code has been enabled in the kernel config. Some architectures currently mimic this by guarding screen_info with ifdef CONFIG_VT or similar. I'd like to make this more flexible. The cost of a few more internal Kconfig tokens seems negligible. > >> Right now, ARCH_HAS_EDID_INFO only works on the old BIOS-based VESA >> systems. In the future, I want to add support for EDID data from EFI and >> OF as well. It would be stored in edid_info. I assume that the new >> symbol will become useful then. > > I don't see why an OF based system would have the same limitation > as legacy BIOS with supporting only a single monitor, if we need > to have a generic representation of EDID data in DT, that would > probably be in a per device property anyway. Sorry that was my mistake. OF has nothing to do with this. > > I suppose you could use FIRMWARE_EDID on EFI or OF systems without > the need for a global edid_info structure, but that would not > share any code with the current fb_firmware_edid() function. The current code is build on top of screen_info and edid_info. I'd preferably not replace that, if possible. Best regards Thomas > > Arnd -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
On Fri, Jun 30, 2023, at 09:46, Thomas Zimmermann wrote: > Am 29.06.23 um 15:21 schrieb Arnd Bergmann: >> On Thu, Jun 29, 2023, at 15:01, Thomas Zimmermann wrote: >>> Am 29.06.23 um 14:35 schrieb Arnd Bergmann: >>>> On Thu, Jun 29, 2023, at 13:45, Thomas Zimmermann wrote: > >>> >>> FIRMWARE_EDID is a user-selectable feature, while ARCH_HAS_EDID_INFO >>> announces an architecture feature. They do different things. >> >> I still have trouble seeing the difference. > > The idea here is that ARCH_HAS_ signals the architecture's support for > the feature. Drivers set 'depends on' in their Kconfig. > > Another Kconfig token, VIDEO_SCREEN_INFO or FIRMWARE_EDID, would then > actually enable the feature. Drivers select VIDEO_SCREEN_INFO or > FIRMWARE_EDID and the architectures contains code like Fair enough. In that case, I guess FIRMWARE_EDID will just depend on ARCH_HAS_EDID_INFO, or possibly "depends on FIRMWARE_EDID || EFI" after it starts calling into an EFI specific function, right? > #ifdef VIDEO_SCREEN_INFO > struct screen_info screen_info = { > /* set values here */ > } > #endif > > This allows us to disable code that requires screen_info/edid_info, but > also disable screen_info/edid_info unless such code has been enabled in > the kernel config. > > Some architectures currently mimic this by guarding screen_info with > ifdef CONFIG_VT or similar. I'd like to make this more flexible. The > cost of a few more internal Kconfig tokens seems negligible. I definitely get it for the screen_info, which needs the complexity. For ARCHARCH_HAS_EDID_INFO I would hope that it's never selected by anything other than x86, so I would still go with just a dependency on x86 for simplicity, but I don't mind having the extra symbol if that keeps it more consistent with how the screen_info is handled. >> I suppose you could use FIRMWARE_EDID on EFI or OF systems without >> the need for a global edid_info structure, but that would not >> share any code with the current fb_firmware_edid() function. > > The current code is build on top of screen_info and edid_info. I'd > preferably not replace that, if possible. One way I could imagine this looking in the end would be something like struct screen_info *fb_screen_info(struct device *dev) { struct screen_info *si = NULL; if (IS_ENABLED(CONFIG_EFI)) si = efi_get_screen_info(dev); if (IS_ENABLED(CONFIG_ARCH_HAS_SCREEN_INFO) && !si) si = screen_info; return si; } corresponding to fb_firmware_edid(). With this, any driver that wants to access screen_info would call this function instead of using the global pointer, plus either NULL pointer check or a CONFIG_ARCH_HAS_SCREEN_INFO dependency. This way we could completely eliminate the global screen_info on arm64, riscv, and loongarch but still use the efi and hyperv framebuffer/drm drivers. Arnd
Hi Arnd Am 30.06.23 um 13:53 schrieb Arnd Bergmann: > On Fri, Jun 30, 2023, at 09:46, Thomas Zimmermann wrote: >> Am 29.06.23 um 15:21 schrieb Arnd Bergmann: >>> On Thu, Jun 29, 2023, at 15:01, Thomas Zimmermann wrote: >>>> Am 29.06.23 um 14:35 schrieb Arnd Bergmann: >>>>> On Thu, Jun 29, 2023, at 13:45, Thomas Zimmermann wrote: >> >>>> >>>> FIRMWARE_EDID is a user-selectable feature, while ARCH_HAS_EDID_INFO >>>> announces an architecture feature. They do different things. >>> >>> I still have trouble seeing the difference. >> >> The idea here is that ARCH_HAS_ signals the architecture's support for >> the feature. Drivers set 'depends on' in their Kconfig. >> >> Another Kconfig token, VIDEO_SCREEN_INFO or FIRMWARE_EDID, would then >> actually enable the feature. Drivers select VIDEO_SCREEN_INFO or >> FIRMWARE_EDID and the architectures contains code like > > Fair enough. In that case, I guess FIRMWARE_EDID will just depend on > ARCH_HAS_EDID_INFO, or possibly "depends on FIRMWARE_EDID || EFI" > after it starts calling into an EFI specific function, right? > >> #ifdef VIDEO_SCREEN_INFO >> struct screen_info screen_info = { >> /* set values here */ >> } >> #endif >> >> This allows us to disable code that requires screen_info/edid_info, but >> also disable screen_info/edid_info unless such code has been enabled in >> the kernel config. >> >> Some architectures currently mimic this by guarding screen_info with >> ifdef CONFIG_VT or similar. I'd like to make this more flexible. The >> cost of a few more internal Kconfig tokens seems negligible. > > I definitely get it for the screen_info, which needs the complexity. > For ARCHARCH_HAS_EDID_INFO I would hope that it's never selected by > anything other than x86, so I would still go with just a dependency > on x86 for simplicity, but I don't mind having the extra symbol if that > keeps it more consistent with how the screen_info is handled. Well, I'd like to add edid_info to platforms with EFI. What would be arm/arm64 and loongarch, I guess. See below for the future plans. > >>> I suppose you could use FIRMWARE_EDID on EFI or OF systems without >>> the need for a global edid_info structure, but that would not >>> share any code with the current fb_firmware_edid() function. >> >> The current code is build on top of screen_info and edid_info. I'd >> preferably not replace that, if possible. > > One way I could imagine this looking in the end would be > something like > > struct screen_info *fb_screen_info(struct device *dev) > { > struct screen_info *si = NULL; > > if (IS_ENABLED(CONFIG_EFI)) > si = efi_get_screen_info(dev); > > if (IS_ENABLED(CONFIG_ARCH_HAS_SCREEN_INFO) && !si) > si = screen_info; > > return si; > } > > corresponding to fb_firmware_edid(). With this, any driver > that wants to access screen_info would call this function > instead of using the global pointer, plus either NULL pointer > check or a CONFIG_ARCH_HAS_SCREEN_INFO dependency. > > This way we could completely eliminate the global screen_info > on arm64, riscv, and loongarch but still use the efi and > hyperv framebuffer/drm drivers. If possible, I'd like to remove global screen_info and edid_info entirely from fbdev and the various consoles. We currently use screen_info to set up the generic framebuffer device in drivers/firmware/sysfb.c. I'd like to use edid_info here as well, so that the generic graphics drivers can get EDID information. For the few fbdev drivers and consoles that require the global screen_info/edid_info, I'd rather provide lookup functions in sysfb (e.g., sysfb_get_screen_info(), sysfb_get_edid_info()). The global screen_info/edid_info state would then become an internal artifact of the sysfb code. Hopefully that explains some of the decisions made in this patchset. Best regards Thomas > > Arnd -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
On Wed, Jul 5, 2023, at 10:18, Thomas Zimmermann wrote: > Am 30.06.23 um 13:53 schrieb Arnd Bergmann: >> On Fri, Jun 30, 2023, at 09:46, Thomas Zimmermann wrote: >>> Am 29.06.23 um 15:21 schrieb Arnd Bergmann: >> >> I definitely get it for the screen_info, which needs the complexity. >> For ARCHARCH_HAS_EDID_INFO I would hope that it's never selected by >> anything other than x86, so I would still go with just a dependency >> on x86 for simplicity, but I don't mind having the extra symbol if that >> keeps it more consistent with how the screen_info is handled. > > Well, I'd like to add edid_info to platforms with EFI. What would be > arm/arm64 and loongarch, I guess. See below for the future plans. To be clear: I don't mind using a 'struct edid_info' being passed around between subsystems, that is clearly an improvement over 'struct screen_info'. It's the global variable that seems like an artifact of linux-2.4 days, and I think we can do better than that. >>>> I suppose you could use FIRMWARE_EDID on EFI or OF systems without >>>> the need for a global edid_info structure, but that would not >>>> share any code with the current fb_firmware_edid() function. >>> >>> The current code is build on top of screen_info and edid_info. I'd >>> preferably not replace that, if possible. >> >> One way I could imagine this looking in the end would be >> something like >> >> struct screen_info *fb_screen_info(struct device *dev) >> { >> struct screen_info *si = NULL; >> >> if (IS_ENABLED(CONFIG_EFI)) >> si = efi_get_screen_info(dev); >> >> if (IS_ENABLED(CONFIG_ARCH_HAS_SCREEN_INFO) && !si) >> si = screen_info; >> >> return si; >> } >> >> corresponding to fb_firmware_edid(). With this, any driver >> that wants to access screen_info would call this function >> instead of using the global pointer, plus either NULL pointer >> check or a CONFIG_ARCH_HAS_SCREEN_INFO dependency. >> >> This way we could completely eliminate the global screen_info >> on arm64, riscv, and loongarch but still use the efi and >> hyperv framebuffer/drm drivers. > > If possible, I'd like to remove global screen_info and edid_info > entirely from fbdev and the various consoles. ok > We currently use screen_info to set up the generic framebuffer device in > drivers/firmware/sysfb.c. I'd like to use edid_info here as well, so > that the generic graphics drivers can get EDID information. > > For the few fbdev drivers and consoles that require the global > screen_info/edid_info, I'd rather provide lookup functions in sysfb > (e.g., sysfb_get_screen_info(), sysfb_get_edid_info()). The global > screen_info/edid_info state would then become an internal artifact of > the sysfb code. > > Hopefully that explains some of the decisions made in this patchset. I spent some more time looking at the screen_info side, after my first set of patches to refine the #ifdefs, and I think we don't even need to make screen_info available to non-x86 drivers at all: - All the vgacon users except for x86 can just register a static screen_info (or simplified into a simpler structure) with the driver itself. This even includes ia64, which does not support EFI framebuffers. - The VESA, vga16, SIS, Intel and HyperV framebuffer drivers only need access to screen_info on x86. HyperV is the only driver that can currently access the data from EFI firmware on arm64, but that is only used for 'gen 1' guests, which I'm pretty sure only exist on x86. - All the other references to screen_info are specific to EFI firmware, so we can move the global definition from arm, arm64, loongarch, riscv and ia64 into the EFI firmware code itself. It is still accessed by efifb and efi-earlycon at this point. I have uploaded version 2 of my series to https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/log/?h=screen-info-v2 and will send it out after I get the green light from build bots. Arnd
diff --git a/arch/Kconfig b/arch/Kconfig index 2f58293fd7bcb..ee9866e4df2b0 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -1466,6 +1466,12 @@ config ARCH_HAS_NONLEAF_PMD_YOUNG address translations. Page table walkers that clear the accessed bit may use this capability to reduce their search space. +config ARCH_HAS_EDID_INFO + bool + help + Selected by architectures that provide a global instance of + edid_info. + config ARCH_HAS_SCREEN_INFO bool help diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index d7c2bf4ee403d..ee81855116be7 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -76,6 +76,7 @@ config X86 select ARCH_HAS_DEBUG_VM_PGTABLE if !X86_PAE select ARCH_HAS_DEVMEM_IS_ALLOWED select ARCH_HAS_EARLY_DEBUG if KGDB + select ARCH_HAS_EDID_INFO select ARCH_HAS_ELF_RANDOMIZE select ARCH_HAS_FAST_MULTIPLIER select ARCH_HAS_FORTIFY_SOURCE diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index d4a72bea56be0..8b2b9ac37c3df 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -21,9 +21,6 @@ config STI_CORE config VIDEO_CMDLINE bool -config ARCH_HAS_SCREEN_INFO - bool - config VIDEO_NOMODESET bool default n diff --git a/include/asm-generic/screen_info.h b/include/asm-generic/screen_info.h index 6fd0e50fabfcd..5efc99c296b40 100644 --- a/include/asm-generic/screen_info.h +++ b/include/asm-generic/screen_info.h @@ -5,6 +5,12 @@ #include <uapi/linux/screen_info.h> +#include <video/edid.h> + +#if defined(CONFIG_ARCH_HAS_EDID_INFO) +extern struct edid_info edid_info; +#endif + #if defined(CONFIG_ARCH_HAS_SCREEN_INFO) extern struct screen_info screen_info; #endif diff --git a/include/video/edid.h b/include/video/edid.h index f614371e9116a..52aabb7060321 100644 --- a/include/video/edid.h +++ b/include/video/edid.h @@ -4,7 +4,4 @@ #include <uapi/video/edid.h> -#ifdef CONFIG_X86 -extern struct edid_info edid_info; -#endif #endif /* __linux_video_edid_h__ */