Linux-fbdev Archive mirror
 help / color / mirror / Atom feed
From: "Arnd Bergmann" <arnd@arndb.de>
To: "Florian Fainelli" <florian.fainelli@broadcom.com>,
	"Daniel Vetter" <daniel@ffwll.ch>
Cc: linux-kernel@vger.kernel.org, "Helge Deller" <deller@gmx.de>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Javier Martinez Canillas" <javierm@redhat.com>,
	"Sam Ravnborg" <sam@ravnborg.org>,
	"open list:FRAMEBUFFER LAYER" <linux-fbdev@vger.kernel.org>,
	"open list:FRAMEBUFFER LAYER" <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH] fbdev: Have CONFIG_FB_NOTIFY be tristate
Date: Wed, 08 May 2024 21:35:38 +0200	[thread overview]
Message-ID: <41639d6b-a429-43f4-8568-12fcd1671cff@app.fastmail.com> (raw)
In-Reply-To: <fe156e32-8ce7-4ce5-99cb-6291ad4b83b0@broadcom.com>

On Wed, May 8, 2024, at 20:37, Florian Fainelli wrote:
> On 5/7/24 04:44, Arnd Bergmann wrote:
>> On Tue, May 7, 2024, at 13:10, Daniel Vetter wrote:
>>> On Mon, May 06, 2024 at 04:53:47PM +0200, Arnd Bergmann wrote:

>> Right, let's wait for Florian to reply. From what he said earlier
>> though, the only reason that the notifiers are getting in the
>> way is the link error you get from trying to load a separately
>> built fb.ko module on a kernel that was built with FB=n / FB_CORE=n,
>> so I don't think he even cares about notifiers, only about
>> allowing the recovery application to mmap() the framebuffer.
>
> Right, we do not really care about notifiers AFAICT. Based upon this 
> discussion there has been an action on our side to stop making use of 
> the FB subsystem for recovery and use the full blow DRM driver instead.

Ok, sounds good.

> While we get there, though I still see some value into this patch (or a 
> v2, that is). I have a v2 ready if you think there is some value in 
> pursuing that route, if not, we can stop there.

I think if you want to do a new version, that is likely to run
into new problems, given that this part of fbdev is particularly
fragile and partly wrong. On the other hand, it would be nice to
have a patch to limit the use of the notifiers to the smallest
set of kernel configs that actually need it, and leave it turned
off for everything else.

These are the ones I could find:

- CONFIG_GUMSTIX_AM200EPD (FB_EVENT_FB_REGISTERED)
- CONFIG_LCD_CORGI, CONFIG_LCD_TDO24M (FB_EVENT_MODE_CHANGE)
- CONFIG_LEDS_TRIGGER_BACKLIGHT (FB_EVENT_BLANK)
- CONFIG_FB_OLPC_DCON (FB_EVENT_BLANK/BL_CORE_FBBLANK)
- CONFIG_FB_SH_MOBILE_LCDC, CONFIG_BACKLIGHT_PCF50633,
  CONFIG_BACKLIGHT_PANDORA, CONFIG_BACKLIGHT_LP855X (BL_CORE_FBBLANK)
- CONFIG_FB_CLPS711X, CONFIG_FB_IMX, CONFIG_MACH_AMS_DELTA
  (lcd BL_CORE_FBBLANK)
- CONFIG_LCD_AMS369FG06, CONFIG_LCD_CORGI, CONFIG_LCD_HX8357,
  CONFIG_LCD_ILI922X, CONFIG_LCD_ILI9320, CONFIG_LCD_HP700,
  CONFIG_LCD_L4F00242T03, CONFIG_LCD_LMS283GF05, CONFIG_LCD_LMS501KF03
  CONFIG_LCD_LTV350QV, CONFIG_LCD_OTM3225A, CONFIG_LCD_PLATFORM,
  CONFIG_LCD_TDO24M (lcd BL_CORE_FBBLANK)

Almost all of these are exclusive to ancient ARMv5 boards or
similar, so if we make the notifiers depend on the whole list,
this would leave it disabled even for most configurations
that enable CONFIG_FB=y.

This could be done with a 'select', but I'd prefer the
'default y; depends on LCD_FOO || LCD_BAR || ...'
variant because that makes it easier to spot if someone
tries to add another one.

      Arnd

  reply	other threads:[~2024-05-08 19:36 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-03 19:28 [PATCH] fbdev: Have CONFIG_FB_NOTIFY be tristate Florian Fainelli
2024-05-03 19:45 ` Arnd Bergmann
2024-05-03 20:22   ` Florian Fainelli
2024-05-06 13:14     ` Daniel Vetter
2024-05-06 14:53       ` Arnd Bergmann
2024-05-06 15:14         ` Arnd Bergmann
2024-05-07 11:10         ` Daniel Vetter
2024-05-07 11:44           ` Arnd Bergmann
2024-05-08 18:37             ` Florian Fainelli
2024-05-08 19:35               ` Arnd Bergmann [this message]
2024-05-08 20:36                 ` Sam Ravnborg
2024-05-08 21:05                   ` Arnd Bergmann
2024-05-04 11:46 ` kernel test robot
2024-05-04 14:42 ` kernel test robot

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=41639d6b-a429-43f4-8568-12fcd1671cff@app.fastmail.com \
    --to=arnd@arndb.de \
    --cc=daniel@ffwll.ch \
    --cc=deller@gmx.de \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=florian.fainelli@broadcom.com \
    --cc=javierm@redhat.com \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sam@ravnborg.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).