08:50 tzimmermann: javierm, hi! do you remember this patch: https://lore.kernel.org/all/20240916110040.1688511-2-javierm@redhat.com/ ?
09:15 javierm: tzimmermann: yes I do, it was to avoid an issue with Coreboot https://lore.kernel.org/all/20240916110040.1688511-3-javierm@redhat.com/
09:17 javierm: when Coreboot had a SeaBIOS payload
09:18 tzimmermann: javierm, i want to replace this: https://elixir.bootlin.com/linux/v6.15/source/drivers/of/platform.c#L588 with a call to sysfb_handles_screen_info(). i was just wondering if that would need an additional type argument
09:18 tzimmermann: but probably not
09:20 javierm: tzimmermann: hmm, I think is the opposite here. You want sysfb to prevent registering a simple-framebuffer device with OF
09:20 javierm: while with Coreboot + SeaBIOS, you want to prevent Coreboot to register the pdev
09:20 tzimmermann: type is one of these values: https://elixir.bootlin.com/linux/v6.15/source/include/uapi/linux/screen_info.h#L51
09:20 javierm: tzimmermann: is screen_info filled with OF ?
09:21 tzimmermann: my reasoning is that sysfb's screen_info comes from the bootloader. the OF nodes likely come from earlier firmware
09:22 tzimmermann: so the boot loader could alter the video mode and make the OF values invalid
09:22 tzimmermann: therefore sysfb/screen_info should always take precence
09:22 javierm: tzimmermann: we had this discussion before and the DT people said that OF values should take precedence
09:22 javierm: let me find the discussion logs
09:22 tzimmermann: really?
09:23 javierm: pretty sure, it rings a bell. Let me find in my email/irc logs
09:38 javierm: tzimmermann: https://lore.kernel.org/all/CAMj1kXG7Xyk0ys9j-XRo7Rr8gYz1qJE8fFSixBOwVbm-pjeX+A@mail.gmail.com/
09:39 javierm: not as I remembered, it seems robher wasn't sure either and Arnd said that the EFI-GOP was too dumb to know about stuff like regulators, clocks, etc and that OF should take precedence
09:39 javierm: *Ard
09:41 javierm: tzimmermann: I don't know what to say :) If you can convince people that OF should skip registering the device and rely on sysfb if a screen_info is filled, I'm OK with that
09:45 tzimmermann: javierm, that argument about regulators/clocks makes some sense
09:49 tzimmermann: i guess i leve it at that. although i'm still a bit worried that this will blow up somehow
09:49 tzimmermann: 'leave'
09:51 tzimmermann: javierm, there's another branch in the OF code for PPC. should we handle sysfb here as well? if so, how? https://elixir.bootlin.com/linux/v6.15/source/drivers/of/platform.c#L539
09:51 arnd: tzimmermann: I think it would be nice to eventually get to making sysfb and screen_info completely x86 specific and remove them from the generic EFI code.
09:52 tzimmermann: arnd, sounds good
09:52 tzimmermann: what would be required?
09:53 javierm: arnd: the problem is when using EFI + ACPI on arm64, so not really x86 specific
09:54 javierm: tzimmermann: I don't is needed because AFAIK on PPC there's no combination with EFI-GOP to cause issues like for arm64
09:54 tzimmermann: ok
09:55 javierm: arnd: sysfb used to be x86 specific but we needed to make it platform independent to support simpledrm without OF on aarch64
09:56 javierm: and even on x86 we have issues due the mentioned Coreboot + SeaBIOS where both firmware pass a sysfb description to the kernel
09:57 javierm: just like happens with DT/OF + u-boot/EFI-GOP
09:58 tzimmermann: javierm, i've been wrapping my head around bringing all this into some coherent design for some time. but it seem slike there's nothing we can do easily
10:00 javierm: same. It's all messy but I couldn't find a way to make it tidy
10:05 tzimmermann: javierm, BTW i've been posting an hdlcd patch this week, which you might want to take a look at
10:06 tzimmermann: javierm, anyway thanks for discussing this issue
10:10 javierm: tzimmermann: reviewed
10:12 arnd: tzimmermann: what are the cases in which sysfb still registers a "efi-framebuffer" device instead of "simple-framebuffer" one? I would have expected all non-x86 platforms to come up with something that is complatible with simple-framebuffer
10:15 javierm: arnd: if someone is using the old efifb driver, since that only matches with an "efi-framebuffer" pdev
10:16 arnd: javierm: I thought it was the other way round: if you have a firmware that advertises a device that is incompatible with simpledrm, then you would need to use efifb
10:17 javierm: arnd: oh, you are right. I was looking now and the new efidrm also matches with "efi-framebuffer"
10:17 arnd: if the problem only exists for users that enable efifb, and it goes away by turning efifb off, couldn't we just make efifb config depend on !SIMPLEDRM
10:20 javierm: arnd: yeah, you are correct. It's a fallback if sysfb_parse_mode() is false and screen_info .orig_video_isVGA == VIDEO_TYPE_EFI
10:21 arnd: javierm: oh, I was looking at a 6.15 source tree, which doesn't have efidrm
10:21 javierm: arnd: yeah, that's in queue for 6.16
10:22 javierm: arnd: and I also wonder what type of platforms would not have a sysfb compatible format while setting VIDEO_TYPE_EFI
10:22 javierm: tzimmermann: do you know? ^
10:23 arnd: I think traditionally it was planar 16-colort vgafb
10:24 javierm: arnd: but wouldn't that set VIDEO_TYPE_EGAC or VIDEO_TYPE_VGAC ?
10:24 arnd: but if now both efidrm and simpledrm use the common sysfb, I would expect them to either both be able to do it or both be incmpatible
10:26 javierm: arnd: indeed
10:26 javierm: once can also force for {efi,vesa}-framebuffer devices to be registered instead of "simple-framebuffer" by disabling CONFIG_SYSFB_SIMPLEFB
10:26 arnd: javierm: it's been too long for me that I looked at this stuff, but my impression was that vga16fb (VIDEO_TYPE_VGAC) is only for the IBM compatible 640x480 resolution, while any higher resolution would have to go through a BIOS initialization or svgalib to set it up.
10:27 javierm: arnd: I was asking because looking at drivers/video/fbdev/vga16fb.c, it checks if either of those types are set and if not it returns -ENODEV
10:28 javierm: so yeah, it seems that unless there's some EFI emulation for a BIOS setting a vga16fb format, the case of incompatible mode + VIDEO_TYPE_EFI will never happen
10:29 tzimmermann: arnd, anything-EFI should work with sysfb
10:30 javierm: tzimmermann: then the fallback is only for the case when CONFIG_SYSFB_SIMPLEFB is disabled?
10:32 tzimmermann: javierm, you can enable SYSFB_SIMPLEFB and boot on a plain-VGA system. that would give vga16fb
10:32 javierm: tzimmermann: yes, but I meant for the EFI case
10:32 tzimmermann: and there are VESA modes that are not supported by simpledrm. booting this load vesadrm/vesafb
10:33 javierm: that is, "efi-framebuffer" will never be registered if SYSFB_SIMPLEFB is enabled
10:33 tzimmermann: for the EFI cases, all modes should be supported by simple-frambuffer
10:33 tzimmermann: so, yes, efidrm/efifb would not be used
10:34 javierm: right. So the fallback in drivers/firmware/sysfb.c is only SYSFB_SIMPLEFB disabled
10:34 javierm: *only for
10:34 tzimmermann: not quite sure i understand. there are still non-EFI systems in use. (?)
10:35 javierm: tzimmermann: the fallback for the EFI case I meant
10:35 tzimmermann: yeah, i don't think you could trigger it
10:36 javierm: https://elixir.bootlin.com/linux/v6.6.36/source/drivers/firmware/sysfb.c#L94
10:36 javierm: this comment is misleading then
10:36 tzimmermann: the comment is indeed misleading
10:36 javierm: but thanks for the explanation, that makes sense
10:37 tzimmermann: TBH i think we should advice against SYSFB_SIMPLEFB=y. with efidrm/vesadrm there will be better alternatives.
10:39 javierm: indeed. It's just that is easier for distros to just built-in simpledrm than also built-in efidrm and vesadrm
10:40 javierm: but simplefb, vesafb and efifb were built-in before, so shouldn't be an issue
11:54 arnd: tzimmermann: I see that sysfb is disabled in of_platform_default_populate_init() when the dtb contains one or more "simple-framebuffer" device, so efidrm should not even be used in that case and this seems to be the reasonable choice to me for embedded systems
11:56 arnd: would it be possible to drop the case of sysfb creating its own "simple-framebuffer" platform device for sysfb instead? That way a kernel can have both efidrm and simpledrm built-in and the default choice becomes the best case in all combinations:
11:56 arnd: - on embedded machines with just one on-chip framebuffer, this gets picked up from the devicetree as simpledrm, while efidrm ignores it
11:58 arnd: - on PC-style machines with PCIe graphics, sysfb registers the framebuffer on the PCIe device, and drivers/video/aperture.c unregisters it while probing the hardware driver
12:00 arnd: It seems like the code in drivers/firmware/sysfb.c that registers the simplefb-framebuffer device was only really needed for DRM-only builds without efifb, but now that efidrm exists, there is no need for that any more.
12:01 arnd: the only change would be for users that used to rely on simplefb to pick up an EFI framebuffer, but they can change to efifb or efidrm
12:06 javierm: arnd: as mentioned, disabling CONFIG_SYSFB_SIMPLEFB already does what you suggest
12:07 javierm: but I think we can't make it unconditionally due some distros only enabling simpledrm instead of {efi,vesa}drm
12:07 tzimmermann: arnd, we have arm machines with DT and grub. they see both sysfb and the OF device. raspi is one
12:08 javierm: tzimmermann: I think is u-boot and not grub that registers the EFI-GOP table? But yes
12:08 tzimmermann: and we have arm machines with only efi/grub, but not DT
12:08 arnd: javierm: sorry, I confused CONFIG_DRM_SIMPLEDRM and CONFIG_SYSFB_SIMPLEFB
12:08 tzimmermann: javierm, right. but you get the idea
12:08 javierm: tzimmermann: yeah
12:09 javierm: arnd: too many CONFIG_*_SIMPLE* :)
12:09 tzimmermann: to reliably detect this, sysfb needs a way to detect simple-framebuffer node from within sysfb. but that's no more trivial than calling sysfb_disable() directly
12:10 arnd: if a machine sees both efidrm and simpledrm, it appears that the code at https://elixir.bootlin.com/linux/v6.15/source/drivers/of/platform.c#L576 does not work as intended
12:11 arnd: maybe the initcall order is wrong and the sysfb device gets created after of_platform_default_populate_init)()?
12:11 javierm: arnd: yeah, I believe that it relies on the init call ordering
12:11 javierm: which is fragile...
12:13 javierm: arnd: sysfb is at device_initcall level while the of is at arch_initcall_sync
12:13 javierm: so the of code will disable sysfb before its init callback
12:14 arnd: javierm: but then sysfb_init() also does "if (disabled) return 0;"
12:15 javierm: arnd: yes, but that's OK because of_platform_default_populate() will register the "simple-framebuffer" device in that case
12:15 arnd: javierm: I mean the "disabled" logic seems to be broken if there are machines that still register the efi sysfb device despite sysfb being disabled early
12:18 javierm: arnd: sorry, I'm not following
12:18 javierm: ff a machine has both a OF "simple-frambuffer" DT node and an EFI-GOP table (that will fill screen_info with type == VIDEO_TYPE_EFI), the former should take precedence
12:19 arnd: javierm: as tzimmermann said above, there are devices that still try to register both, e.g. the raspi with u-boot/uefi
12:19 javierm: in that case, the of platform code will call sysfb_disable(), register the "simple-framebuffer" and sysfb_init() be a no-op
12:20 arnd: yes, I see in the code that this is what is supposed to happen, yet tzimmermann said it fails to do that
12:20 javierm: arnd: oh, I see. Yes that happened and was reported as a bug; and was the reason we added that sysfb_disable()
12:22 javierm: similarly, coreboot with a seabios payload had the same issue and in that case coreboot avoids creating a "simple-framebuffer" device and lets sysfb to do it
12:22 javierm: because were told that seabios had precedence over coreboot
12:25 javierm: that's why I said that's really a mess and I don't know how to make it more consistent
12:32 tzimmermann: arnd, i think you misunderstood. the current code works correctly
12:32 tzimmermann: it's just very confusing
12:43 zmike: jenatali: can you look into https://gitlab.freedesktop.org/mesa/mesa/-/jobs/77410688
12:45 jenatali: Yeah, in a few hours
12:45 zmike: thx
13:10 arnd: tzimmermann: I think I got it now. How about flipping the dependencies for SYSFB_SIMPLEFB to reflect the new recommendation then? https://www.irccloud.com/pastebin/8logjfIs/
13:12 arnd: I've tried to update the help text a bit, but it's still confusing because it was all originally written with the intention of obsoleting efifb/vesafb in favor of simplefb, but now we recommend efidrm/vesadrm instead of simpledrm
13:21 tzimmermann: arnd, the change to SYSFB_SIMPLEFB might break something. ok for DRM_EFIDRM and DRM_VESADRM. FB_EFI shout depend on (!EFIDRM || COMPILE_TEST); likewise for FB_VESA. the new text in the first paragraph sounds off
13:29 arnd: tzimmermann: how would the SYSFB_SIMPLEFB change break anything? The dependency on '!(DRM_EFIDRM || DRM_VESADRM)' is just equivalent to the removed dependency, while the added dependency on 'FB_SIMPLE || DRM_SIMPLEDRM' only guards against a case that was already broken
13:31 tzimmermann: arnd, there could display modes that are suported by vesa, but not by simplefb. the code should likely fallback to vesadrm in that case.
13:31 tzimmermann: and FB_VESA and FB_EFI are still a thing on some systems, so you'd have to check for those as well.
13:31 tzimmermann: i'd just leave this out
13:55 arnd: tzimmermann: I'm still not following what you mean here. If there is a mode that is supported by vesadrm but not simpledrm, that is currently broken when SYSFB_SIMPLEFB is enabled, because DRM_VESADRM depends on !SYSFB_SIMPLEFB. With my patch, turning on DRM_VESADRM forces SYSFB_SIMPLEFB to be disabled. Regardless of my patch, the vesa framebuffer works with DRM_VESADRM=y and SYSFB_SIMPLEFB=n.
13:57 tzimmermann: arnd, could be. vesadrm is still new. not all edge cases have been tested. i meant, i just would not overthink the dependencies of SYSFB_SIMPLEFB
13:58 arnd: the fallback is something that vesafb/efifb users may depend on, so existing configs with DRM_SIMPLEDRM=y, FB_VESA=y, FB_EFI=y, SYSFB_SIMPLEFB=y should ideally remain possible
13:59 arnd: I think it would be good if anyone starting to use vesadrm/efidrm now is forced to build with SYSFB_SIMPLEFB=n, so that by the time we remove vesafb/efifb, SYSFB_SIMPLEFB can also be removed
14:00 arnd: i.e. not offer too many new combinations that we want to eventually remove again
14:01 javierm: the idea then is to not allow simpledrm to be used with EFI and VESA/VGA anymore ?
14:07 tzimmermann: arnd, well, ok then
14:07 arnd: javierm: yes, based on what tzimmermann suggested earlier "TBH i think we should advice against SYSFB_SIMPLEFB=y. with efidrm/vesadrm there will be better alternatives"
14:10 arnd: the old vesafb/efifb drivers are not very nice drivers, so the idea of sysfb_simplefb to use the more modern simplefb made sense at the time. Similarly, using simpledrm instead of simplefb is another step forward. With efidrm/vesadrm, I think the old issues of efifb/vesafb no longer apply, so sysfb_simplefb has served its purpose and can eventually get retired
14:20 tzimmermann: arnd, the simple* drivers are a bit overloaded. they handle DT's simple-framebuffer and the kernel-internal simplefb_platform_data. it would be nice to have two separate drivers at some point.
14:21 javierm: arnd: that's true. But then as tzimmermann said, we should make simpledrm only about "simple-framebuffer" devices and drop sysfb at all
14:21 javierm: either we want to have the convenience of a driver that could use any generic system framebuffer provided by firmware or have dedicated drivers for each firmware interface
14:22 tzimmermann: simplefb_platform_data is currently required by the coreboot framebuffer and the N64 framebuffer. they could likely be migrated to some sort of plain-framebuffer device (instead of being lumped together with simple-framebuffer)
14:22 tzimmermann: no hurries though
14:22 javierm: tzimmermann: simplesysfbdrm :D
14:22 tzimmermann: :D
14:23 javierm: tzimmermann: but yeah, I remember when you introduced ofdrm that I mentioned about removing the of-specific bits from simpledrm
14:24 tzimmermann: i'm pretty sure the n64 requires an fbdev driver
14:24 javierm: and you explained to me that ppc of and DT of were different
14:24 tzimmermann: coreboot could certainly go with drm
14:25 javierm: tzimmermann: it feels wrong that simpledrm needs to know about DT properties parsing
14:25 javierm: so having a dedicated driver for DT "simple-framebuffer" sounds better indeed
14:25 tzimmermann: but isn't that what all drivers do?
14:26 tzimmermann: parse the DT?
14:27 javierm: tzimmermann: yeah, but simpledrm was supposed to be a driver that use generic system provided framebuffers
14:27 javierm: you mentioned that we could split in two drivers, and in that case the driver that would just use simplefb_platform_data will be platform agnostic
14:27 tzimmermann: provided by the DT
14:28 arnd: I think it makes sense to have the "simpledrm" driver be the one that parses DT properties for a device that is compatible with the "simple-framebuffer" binding, but that doesn't have to be the same driver that uses simplefb_platform_data
14:28 javierm: tzimmermann: I tried to say what arnd just mentioned
14:28 tzimmermann: simplefb_platform_data would become someinthg like pixelbuf_data
14:29 javierm: tzimmermann: you would have the platform OF code to register a "simple-framebuffer" and match with a simpledrm driver that parses DT properties
14:29 javierm: tzimmermann: and then sysfb will register a device (that won't be named "simple-framebuffer" anymore, but "generic-framebuffer" or something)
14:29 javierm: that will match with a sysfbdrm that won't know anything about FW interfaces
14:29 javierm: just use whatever is provided from screen_info
14:30 tzimmermann: sort of
14:30 arnd: drivers/firmware/google/framebuffer-coreboot.c converts the google specific data into simplefb_platform_data, but that file could easily be moved into drivers/gpu/drm/sysfb next to simpledrm/efidrm/vesadrm/ofdrm
14:30 tzimmermann: i think we should try to kill SYSFB_SIMPLEFB first
14:30 tzimmermann: then we'd not need the generic-framebuffer in sysfb
14:31 tzimmermann: it would be a thing of n64 and coreboot
14:31 arnd: mips/n64 can create device_property entries for its platform device to use the same probe function as the dtb case
14:32 tzimmermann: there's been talk about keeping the framebuffer across kexec/kdump. it could be build using the generic-framebuffer
14:32 javierm: arnd: interesting idea
14:33 arnd: tzimmermann: do you have an estimate about when we might consider removing the old efifb/vesafb? do you think it's worth removing sysfb_simplefb any earlier than that, or just remove them all at the same time?
14:34 arnd: also the old simplefb, I guess
14:35 javierm: arnd: I would first remove the {efi,vesa}fb in a few kernel releases, to give people time to migrate to {efi,vesa}drm
14:35 javierm: simplefb could really be removed IMO. I don't see a reason to keep it around anymore
14:36 tzimmermann: arnd, no earlier than 2 yrs from now
14:36 tzimmermann: still needs work and needs to make it to the distros for testing.
14:36 tzimmermann: and it's not really high prio
14:37 javierm: tzimmermann: I always wondered what is the policy of fbdev drivers removal
14:38 javierm: in practice I would guess that most distros already migrated from simple/efi/vesa fbdev drivers to simpledrm for some time
14:38 tzimmermann: javierm, there's always someone who appears to be using the old code. so it rarely happens
14:39 tzimmermann: i tried to remove udlfb. someone complained, yet we have a much better udl driver in drm
14:39 javierm: tzimmermann: yeah, that's the reason why I didn't attempt to remove the old ssd1307fb driver
14:40 javierm: even when the ssd130x drm driver now supports different families, I2C and SPI
14:40 tzimmermann: and most of the really old HW is hard to support in drm. the fbdev drivers have less overhead. that makes a difference on the old platforms
14:40 arnd: I see that the armv7 defconfig (arch/arm/configs/multi_v7_defconfig) still contains simplefb and efifb, but not simpledrm, we should probably fix that
14:40 tzimmermann: arnd, great
14:43 javierm: tzimmermann: right. That's why I never proposed removed an fbdev driver, but still said that "I would remove it" :)
14:44 tzimmermann: javierm, maybe we could move some drivers to staging?
14:45 tzimmermann: or make them depend on !DRM
14:46 javierm: tzimmermann: moving to staging makes sense. It seems that's the deprecation path that others drivers followed
15:19 jenatali: zmike: About to take a look. Care to trade for a review on https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/35247? ;)
15:20 zmike: jenatali: for you, I would've just reviewed if you asked
15:21 jenatali: Thanks. After a VS update I can't actually build without that MR so it'll save me some effort to not have to rebase everything on top of it
15:21 zmike: ... in like 30 mins tho cuz I'm meeting with some vip clients atm
15:21 jenatali: Fair
15:21 daniels: IHOP Fridays is it
15:23 zmike: https://usercontent.irccloud-cdn.com/file/aO2cmICZ/20250530_112301.jpg
15:38 jenatali: zmike: Debugged. TC is broken
15:38 zmike: wut
15:38 zmike: impossible
15:38 jenatali: It tries to refcount surfaces which are no longer heap allocated
15:39 zmike: gasp
15:39 zmike:searches for someone to blame
15:39 jenatali: Insert spiderman pointing meme
17:02 zmike: is d3d10umd still maintained?
17:10 glehmann: deleting anything that uses tgsi is always a good idea if you ask me
17:10 zmike: I agree
17:11 jenatali: Might want to ping the VMWare/Broadcom folks directly?
17:11 zmike: I created a ticket
17:44 zmike: alright that's probably enough code deletion for the week
17:44 HdkR: But what about weekend code deletion?
17:45 zmike: no that's when I do brain deletion
17:45 HdkR: Ah, time honoured tradition that
20:54 mareko: TGSI practically doesn't exist anymore
20:55 mareko: for many drivers at least
20:55 mareko: not all drivers