All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <linux@treblig.org>
To: Randy Dunlap <rdunlap@infradead.org>
Cc: sam@ravnborg.org, linux-kernel@vger.kernel.org,
	npiggin@gmail.com, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v2] powerpc: Use shared font data
Date: Tue, 8 Aug 2023 23:02:22 +0000	[thread overview]
Message-ID: <ZNLJfgD5HDz3yief@gallifrey> (raw)
In-Reply-To: <8599e99b-ad80-abf8-ad40-4cb8262f047a@infradead.org>

* Randy Dunlap (rdunlap@infradead.org) wrote:
> 
> 
> On 8/8/23 10:28, Dr. David Alan Gilbert wrote:
> > * Randy Dunlap (rdunlap@infradead.org) wrote:
> >> Hi--
> >>
> >> On 8/6/23 18:07, linux@treblig.org wrote:
> >>> From: "Dr. David Alan Gilbert" <linux@treblig.org>
> >>>
> >>> PowerPC has a 'btext' font used for the console which is almost identical
> >>> to the shared font_sun8x16, so use it rather than duplicating the data.
> >>>
> >>> They were actually identical until about a decade ago when
> >>>    commit bcfbeecea11c ("drivers: console: font_: Change a glyph from
> >>>                         "broken bar" to "vertical line"")
> >>>
> >>> which changed the | in the shared font to be a solid
> >>> bar rather than a broken bar.  That's the only difference.
> >>>
> >>> This was originally spotted by PMD which noticed that sparc does
> >>> the same thing with the same data, and they also share a bunch
> >>> of functions to manipulate the data.  I've previously posted a near
> >>> identical patch for sparc.
> >>>
> >>> One difference I notice in PowerPC is that there are a bunch of compile
> >>> options for the .c files for the early code to avoid a bunch of security
> >>> compilation features;  it's not clear to me if this is a problem for
> >>> this font data.
> >>>
> >>> Tested very lightly with a boot without FS in qemu.
> >>>
> >>> v2
> >>>   Added 'select FONT_SUPPORT' (to stop modconfig causing the font to be
> >>>    linked into a module rather than the main kernel)
> >>>   Added 'select FONTS' to satisfy requirements in lib/fonts
> >>>
> >>> Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
> >>> ---
> >>>  arch/powerpc/Kconfig.debug  |   3 +
> >>>  arch/powerpc/kernel/btext.c | 360 +-----------------------------------
> >>>  2 files changed, 9 insertions(+), 354 deletions(-)
> >>>
> >>> diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
> >>> index 2a54fadbeaf51..521c4baf30e88 100644
> >>> --- a/arch/powerpc/Kconfig.debug
> >>> +++ b/arch/powerpc/Kconfig.debug
> >>> @@ -147,6 +147,9 @@ config BDI_SWITCH
> >>>  config BOOTX_TEXT
> >>>  	bool "Support for early boot text console (BootX or OpenFirmware only)"
> >>>  	depends on PPC_BOOK3S
> >>> +	select FONT_SUN8x16
> >>> +	select FONT_SUPPORT
> >>> +	select FONTS
> >>>  	help
> >>>  	  Say Y here to see progress messages from the boot firmware in text
> >>>  	  mode. Requires either BootX or Open Firmware.
> >>
> >> kconfig tells me:
> >>
> >> WARNING: unmet direct dependencies detected for FONTS
> >>   Depends on [n]: FONT_SUPPORT [=y] && (FRAMEBUFFER_CONSOLE [=n] || STI_CONSOLE [=n])
> >>   Selected by [y]:
> >>   - BOOTX_TEXT [=y] && PPC_BOOK3S [=y]
> >>
> >> WARNING: unmet direct dependencies detected for FONT_SUN8x16
> >>   Depends on [n]: FONT_SUPPORT [=y] && FRAMEBUFFER_CONSOLE [=n] && (!SPARC && FONTS [=y] || SPARC)
> >>   Selected by [y]:
> >>   - BOOTX_TEXT [=y] && PPC_BOOK3S [=y]
> >>
> >> because FONTS depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE and neither of those is set.
> > 
> > I'm not getting the warnings in the v2, with a few configs; what command
> > are using?
> > 
> 
> My 'make' build target is either pp32_randconfig or ppc64_randconfig.
> I see kconfig warnings in > 50% of the randconfigs. (small sample size,
> around 20)

Thanks, that triggers it for me; I'll have a discussion with it....

Dave

> 
> > I'm tempted to change the FONT_SUN8x16 dependency line to have
> > SPARC||BOOTX_TEXT or SPARC||POWERPC  and drop the 'select FONTS' I
> > added.
> > 
> > Dave
> > 
> >>
> >> -- 
> >> ~Randy
> 
> -- 
> ~Randy
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

WARNING: multiple messages have this Message-ID (diff)
From: "Dr. David Alan Gilbert" <linux@treblig.org>
To: Randy Dunlap <rdunlap@infradead.org>
Cc: mpe@ellerman.id.au, npiggin@gmail.com,
	christophe.leroy@csgroup.eu, sam@ravnborg.org,
	benh@kernel.crashing.org, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] powerpc: Use shared font data
Date: Tue, 8 Aug 2023 23:02:22 +0000	[thread overview]
Message-ID: <ZNLJfgD5HDz3yief@gallifrey> (raw)
In-Reply-To: <8599e99b-ad80-abf8-ad40-4cb8262f047a@infradead.org>

* Randy Dunlap (rdunlap@infradead.org) wrote:
> 
> 
> On 8/8/23 10:28, Dr. David Alan Gilbert wrote:
> > * Randy Dunlap (rdunlap@infradead.org) wrote:
> >> Hi--
> >>
> >> On 8/6/23 18:07, linux@treblig.org wrote:
> >>> From: "Dr. David Alan Gilbert" <linux@treblig.org>
> >>>
> >>> PowerPC has a 'btext' font used for the console which is almost identical
> >>> to the shared font_sun8x16, so use it rather than duplicating the data.
> >>>
> >>> They were actually identical until about a decade ago when
> >>>    commit bcfbeecea11c ("drivers: console: font_: Change a glyph from
> >>>                         "broken bar" to "vertical line"")
> >>>
> >>> which changed the | in the shared font to be a solid
> >>> bar rather than a broken bar.  That's the only difference.
> >>>
> >>> This was originally spotted by PMD which noticed that sparc does
> >>> the same thing with the same data, and they also share a bunch
> >>> of functions to manipulate the data.  I've previously posted a near
> >>> identical patch for sparc.
> >>>
> >>> One difference I notice in PowerPC is that there are a bunch of compile
> >>> options for the .c files for the early code to avoid a bunch of security
> >>> compilation features;  it's not clear to me if this is a problem for
> >>> this font data.
> >>>
> >>> Tested very lightly with a boot without FS in qemu.
> >>>
> >>> v2
> >>>   Added 'select FONT_SUPPORT' (to stop modconfig causing the font to be
> >>>    linked into a module rather than the main kernel)
> >>>   Added 'select FONTS' to satisfy requirements in lib/fonts
> >>>
> >>> Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
> >>> ---
> >>>  arch/powerpc/Kconfig.debug  |   3 +
> >>>  arch/powerpc/kernel/btext.c | 360 +-----------------------------------
> >>>  2 files changed, 9 insertions(+), 354 deletions(-)
> >>>
> >>> diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
> >>> index 2a54fadbeaf51..521c4baf30e88 100644
> >>> --- a/arch/powerpc/Kconfig.debug
> >>> +++ b/arch/powerpc/Kconfig.debug
> >>> @@ -147,6 +147,9 @@ config BDI_SWITCH
> >>>  config BOOTX_TEXT
> >>>  	bool "Support for early boot text console (BootX or OpenFirmware only)"
> >>>  	depends on PPC_BOOK3S
> >>> +	select FONT_SUN8x16
> >>> +	select FONT_SUPPORT
> >>> +	select FONTS
> >>>  	help
> >>>  	  Say Y here to see progress messages from the boot firmware in text
> >>>  	  mode. Requires either BootX or Open Firmware.
> >>
> >> kconfig tells me:
> >>
> >> WARNING: unmet direct dependencies detected for FONTS
> >>   Depends on [n]: FONT_SUPPORT [=y] && (FRAMEBUFFER_CONSOLE [=n] || STI_CONSOLE [=n])
> >>   Selected by [y]:
> >>   - BOOTX_TEXT [=y] && PPC_BOOK3S [=y]
> >>
> >> WARNING: unmet direct dependencies detected for FONT_SUN8x16
> >>   Depends on [n]: FONT_SUPPORT [=y] && FRAMEBUFFER_CONSOLE [=n] && (!SPARC && FONTS [=y] || SPARC)
> >>   Selected by [y]:
> >>   - BOOTX_TEXT [=y] && PPC_BOOK3S [=y]
> >>
> >> because FONTS depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE and neither of those is set.
> > 
> > I'm not getting the warnings in the v2, with a few configs; what command
> > are using?
> > 
> 
> My 'make' build target is either pp32_randconfig or ppc64_randconfig.
> I see kconfig warnings in > 50% of the randconfigs. (small sample size,
> around 20)

Thanks, that triggers it for me; I'll have a discussion with it....

Dave

> 
> > I'm tempted to change the FONT_SUN8x16 dependency line to have
> > SPARC||BOOTX_TEXT or SPARC||POWERPC  and drop the 'select FONTS' I
> > added.
> > 
> > Dave
> > 
> >>
> >> -- 
> >> ~Randy
> 
> -- 
> ~Randy
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

  reply	other threads:[~2023-08-08 23:03 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-07  1:07 [PATCH v2] powerpc: Use shared font data linux
2023-08-07  1:07 ` linux
2023-08-08 16:09 ` Randy Dunlap
2023-08-08 17:28   ` Dr. David Alan Gilbert
2023-08-08 17:28     ` Dr. David Alan Gilbert
2023-08-08 20:20     ` Randy Dunlap
2023-08-08 20:20       ` Randy Dunlap
2023-08-08 23:02       ` Dr. David Alan Gilbert [this message]
2023-08-08 23:02         ` Dr. David Alan Gilbert

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=ZNLJfgD5HDz3yief@gallifrey \
    --to=linux@treblig.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=npiggin@gmail.com \
    --cc=rdunlap@infradead.org \
    --cc=sam@ravnborg.org \
    /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.