06:37 javierm: tzimmermann: jwerner from Google suggests that in Coreboot systems, the Coreboot table isn't the single source of truth, in contrast to DT systems so the same solution doesn't apply
06:38 javierm: tzimmermann: for example, one can use edk2/UEFI as Coreboot payload (https://doc.coreboot.org/payloads.html#edk2) in that case the payload can decide to pass a different mode in the EFI-GOP table
06:39 javierm: or SeaBIOS a VESA mode as is the case on the reported bug. I've proposed a different patch then that just checks whether the screen_info video type is VLFB or EFI and bail in that case
06:40 tzimmermann: javierm, i see. let me read and proces that email. my morning coffee has yet to kick in :)
06:42 javierm: tzimmermann: :)
06:46 tzimmermann: javierm, makes sense to me
06:46 tzimmermann: let me comment on the actual patch
06:54 tzimmermann: javierm, thanks a lot for handling the bug report
06:54 javierm: tzimmermann: you are welcome
08:14 tzimmermann: jfalempe_, hi, have a minute?
08:20 jfalempe: tzimmermann: Hi, yes. Sorry due to network glitch I wasn't registered.
08:20 tzimmermann: first, thanks for the ast reviews
08:21 jfalempe: you're welcome :)
08:22 tzimmermann: jfalempe, do you know anything about the tx chips that are listed at https://elixir.bootlin.com/linux/v6.10/source/drivers/gpu/drm/ast/ast_reg.h#L45
08:22 tzimmermann: ?
08:23 jfalempe: unfortunately no, I don't know the internal of aspeed chips.
08:24 jfalempe: looks like only ASTDP_DPMCU_TX is used.
08:24 tzimmermann: these constants can be read from vgacrd1
08:24 tzimmermann: so i assume that each of them is meaningful in some way
08:24 tzimmermann: right, astdp is used
08:25 tzimmermann: there's also sil164
08:25 tzimmermann: dp501 and embedded_fw
08:25 tzimmermann: but some are mysterious
08:25 tzimmermann: for example that ite chip does HDMI
08:26 tzimmermann: CH7003 appears to be a TV output (?)
08:26 tzimmermann: i'm not even aware of such ast devices
08:27 jfalempe: Yes, it might refer to some internal products that were never release.
08:27 tzimmermann: jfalempe, right now, these odd cases seem to be reported as vga or sil164. when refactoring, should i make it fail? or something else?
08:27 tzimmermann: i'm slightly confused about this
08:29 jfalempe: I'm not sure either. There is tradeoff between following the spec and having clean code, and not breaking things that already works.
08:30 tzimmermann: i guess i could make something up
08:30 jfalempe: Maybe put a warning first, if we get unconsistent value for this register, and wait a bit before failing.
08:32 tzimmermann: in some cases, the driver looks at the hightes bit in vgacra3 to detect analog (vga) from digital (sil164) outputs. i'd assume that this implicitly covers these odd chips as well
08:32 tzimmermann: such a test is at https://elixir.bootlin.com/linux/v6.10/source/drivers/gpu/drm/ast/ast_main.c#L86
08:33 tzimmermann: the ite chip is likely digital (hdmi) and would be handled like the sil164
08:33 tzimmermann: and the ch7003 (tv) is likely analog and would be handled as vga
08:35 tzimmermann: and it works because these tx chips are programmed by ast's firmware. driver support doesn't seem to be required.
08:36 tzimmermann: (if they even exist in real devices)
08:38 jfalempe: I think I would first merge a patch that warn if this register reports values that would break, and wait a few release before using it for real.
08:41 tzimmermann: jfalempe, ok. what kind of warning would be appropriate? i'd like to here from people that own such a device. so maybe a drm_WARN_ON() for the odd values?
08:41 jfalempe: tzimmermann: yes a real kernel warning, so that people will notice.
08:42 tzimmermann: ok, great. i'll send out a patch to do that
08:42 tzimmermann: thanks a lot
08:43 jfalempe: When a hw register is not used by the software, it tends to be less reliable, so at least we will be careful on this.
14:09 MrCooper: hmm, seeing a lot of cycles spent in mesa_db_update_index during a piglit run
14:25 jfalempe: tzimmermann: I'm trying to rebase drm_log with your series with the client setup helper. One issue is that drm_client_setup.o is part of the kms helper module.
14:26 tzimmermann: jfalempe, is there a linker conflict?
14:26 jfalempe: drm_log and the drm client api is part of the drm module, so it would be better to have the setup here too ?
14:27 jfalempe: I think it's just that it makes drm_log depends on kms helper, even if it doesn't need to.
14:28 tzimmermann: drm_client calls into the fbdev client, which is in drm_kms_helper
14:28 tzimmermann: drm_client_setup calls into the fbdev client, which is in drm_kms_helper
14:29 jfalempe: ok, I though it wouldn't work, because kms helper also depends on drm.
14:29 tzimmermann: the thing is: most of the client code is called from drm drivers. so it should be in a selectable module
14:29 tzimmermann: only a small part of the client api is called from the drm core
14:29 tzimmermann: this needs to go into drm.ko
14:29 tzimmermann: i think
14:30 tzimmermann: but that is a bit of a rework of the dependencies; so it belongs into a separate series
14:30 tzimmermann: why does drm_log need ot be in drm.ko ?
14:31 jfalempe: yes, it can be done as a separate step
14:32 jfalempe: drm_log is to show the early boot log, it doesn't make sense to have it as a module, because at that time your userspace could do the same.
14:32 tzimmermann: if you need a short-term solution; i'd suggest to link drm_log into kms-helpers. we should likely add a drm_client.ko module that contains the client code
14:33 tzimmermann: but all your drivers are in modules. you cannot run drm_log before the driver started
14:33 jfalempe: simpledrm usually is built-in
14:33 vsyrjala: is there anything that doesn't actually depend on drm_kms_helper? just wondering why we insist on hanging on to it and getting things messed up all the time
14:34 tzimmermann: that's not my the point. even if simpledrm is linked in. it is still a separate module
14:34 tzimmermann: i915 didn't use kms-helper. but i'm not sure these days
14:35 tzimmermann: but kms helper is something else then drm code. even if everything uses it
14:35 jfalempe: and kms helper are usually built as a module in distribution.
14:36 tzimmermann: likely not
14:36 vsyrjala: even drm_rect seems to be in kms_helper these days, and everyone should depend on that on account of plane clipping
14:36 jfalempe: hum no, it's not in fedora, I think I was confused by other helper modules.
14:36 vsyrjala: i would think a full kms vs. no kms would be a better split that some arbitrary "helper" vs. "not helper" idea
14:39 tzimmermann: vsyrjala, drm.ko is for core code. kms_helper.ko is for implementing drivers. it's two separate things. how do you split full-kms vs no-kms?
14:40 tzimmermann: well, drivers/accel does not depend on kms helpers
14:41 tzimmermann: jfalempe, we should make a client module that has all the clients
14:42 jfalempe: looking at the dep chain, simpledrm depends on kms helper, so if simpledrm is built-in, kms help would be too.
14:43 tzimmermann: yes
14:44 tzimmermann: but drm_log is not part of the DRM core api. it should not be part of drm.ko
14:45 tzimmermann: and this will likely break as soon as someone builds simpledrm as a module
14:45 jfalempe: It doesn't call directly simpledrm, so should be safe, but I agree it's not a core drm api.
14:47 tzimmermann: because drm_client is also in drm.ko, i assume (?)
14:48 vsyrjala: does anyone have an actual defintion for "core code"? "code that can be called via standard entrypoints without going through a driver" maybe?
14:48 tzimmermann: that's why i said that there should be a drm_client.ko module that contains all the client code.
14:49 tzimmermann: vsyrjala, my definition is that core is anything that is required for ioctls to work
14:49 jfalempe: Just looking at my drm_log patch, it is a separate "module" but it's a bool, so cannot be built as a .ko
14:52 javierm: jfalempe, tzimmermann: IMO drm_log should be part of drm (which should be built-in along simpledrm) since we want that output even when initrd fails to be loaded (which could contain the DRM modules)
14:52 javierm: unsure about the drm_client module dep though
14:52 tzimmermann: jfalempe, drm used to be quite large whne linked in. i've spend several patch series on reducing the impact of drm modules and making it possible to like them as modules if wanted. it's usually better to split things up into modules and let the kconfig figure out linking.
14:53 javierm: tzimmermann: yes, agree. jfalempe why is drm_log a bool and not tristate ?
14:53 tzimmermann: javierm, iff you select a driver to be built-in it's dependenceies will be built-in as well. but why make this mandatory. let the client code sit in a separate module and let kconfig decide when to link it into the kernel
14:54 jfalempe: for drm_log it doesn't make sense as a module, but I think it should still work
14:54 javierm: tzimmermann: yes, completely agree. Most distros would set drm, simpledrm, drm_client and drm_log as =y but others could have it as a module
14:55 tzimmermann: jfalempe, you're confusing general kernel configuration and your specific usecase
14:55 javierm: jfalempe: I think that's the distinction that tzimmermann is making and makes sense indeed. Forcing vs allowing users to choose
14:56 jfalempe: I think the reason is that I call drm_log_register() from the main drm_register() code
14:56 javierm: jfalempe: maybe a compromise is maing it tristate but default y ?
14:56 tzimmermann: most distros will make simpledrm=y and have alomost everything built in. but not everyone whants that
14:56 javierm: tzimmermann: yeah
14:56 tzimmermann: jfalempe, drm_log_register() should later go into drm_client_setup()
14:57 jfalempe: but with the client setup patch from tzimmermann, that is not needed anymore, so drm_log can be built as a module.
14:57 tzimmermann: right
14:57 jfalempe: so I just need to select kms_helper from drm_log and that should be ok.
14:58 javierm: jfalempe: cool, then yes I agree that making it tristate is better and let distros chose their default
14:58 javierm: but if someone doesn't want drm_log, then shouldn't increase their drm.o size
14:58 tzimmermann: jfalempe, what exactly does drm_log_register() do?
14:59 jfalempe: it calls drm_client_init(), drm_client_register() and register_console()
15:01 tzimmermann: all this should then happen from drm_client_setup()
15:02 jfalempe: yes, that's what I'm doing, adding a kernel module parameter to choose the drm client, between fbdev or drm log
15:04 tzimmermann: for testing, drm_log should go into kms-helpers. just like fbdev
15:05 tzimmermann: and before you merge it we should have a drm_client.ko module that contains all the clients
15:05 tzimmermann: so your kernel parameter can be something like drm_client.default=log (i'm just making up some names)
15:06 tzimmermann: or drm_client.default=fbdev
15:07 tzimmermann: alternatively, we could split drm_client and fbdev into it's own drm_client.ko *now*
15:07 tzimmermann: before i merge drm_client_setup()
15:08 jfalempe: Yes, either looks good to me.
15:09 jfalempe: as you have a lot of reviews, maybe merge your long series first
15:09 jfalempe: then we can split drm_client and fbdev into it's own drm_client module.
15:10 jfalempe: and then merge drm_log on top of that.
15:11 jfalempe: We have plenty of time before the next merge window, so no need to rush.
15:12 tzimmermann: ok, that would work. i'll also try to come up with some RFC patches that split off client code, so that we have some actual cod eto discuss. (IRC discussions are stressful and confusing to me)
15:12 tzimmermann: it's friday afternoon here. see you next week
15:13 jfalempe: tzimmermann: thanks that's fine.
15:13 javierm: tzimmermann: have a nice weekend
15:31 MrCooper: yikes, piglit gpu takes ~20 minutes with the default Mesa-DB shader cache, just 7:30 minutes with MESA_DISK_CACHE_MULTI_FILE=1
15:47 ishitatsuyuki: any idea why? does it slows down driver initialization?
15:47 ishitatsuyuki: hmm, iirc it does scan over all the hash indexes on startup, so maybe it's O(n^2) if you reinitialize a lot
15:49 ishitatsuyuki: disabling it for piglit sounds like the short-term workaround
15:50 ishitatsuyuki: maybe we should have went for a sqlite db as the format after all :/
15:59 MrCooper: yeah, it burns a lot of CPU cycles reading the cache indices
15:59 MrCooper: and initializing the hash tables
16:02 MrCooper: looks like CI is "lucky" in that it still preserves only the old mesa_shader_cache directory between jobs, so the Mesa-DB cache always starts empty
16:02 MrCooper: seems like the Mesa-DB cache really shouldn't be enabled by default yet though
16:16 DemiMarie: SQLite is great so long as one either does not use NFS home directories or has a way to prevent the same user logging in on multiple machines at once. I suspect Mesa is often used on compute clusters where both assumptions fail miserably.
16:26 daniels: MrCooper: you think it would be better with MULTI_FILE?
16:26 daniels: having the cache is definitely a win over no cache
16:27 daniels: or at least, it was when we measured it
16:31 MrCooper: daniels: not sure about all the ramifications yet, but .gitlab-ci/deqp-runner.sh & .gitlab-ci/piglit/piglit-runner.sh still use tmpfs for ${SHADER_CACHE_HOME}/mesa_shader_cache, which is the multi-file cache directory
18:41 Ermine: Do I understand correctly that simpledrm is implemented on top of fbdev?
18:44 javierm: Ermine: no, it's the opposite. Using the DRM fbdev emulation layer you could have an fbdev on top of simpledrm
19:09 Ermine: javierm: ok, thank you
19:22 llyyr: could someone take a look at !31122? Should be relatively simple
20:59 Lyude: I assume you can have multiple read-only mappings to a GEM object yeah?
21:30 emersion: i don't see a reason why not
22:01 Lyude: yay
22:04 Lyude: Not sure if I'll have the time before XDC, but I -think- I might have a general idea now of how I can implement CRC generation for rvkms