Hi

Am 17.01.26 um 02:38 schrieb Julius Werner:
Corebootdrm already depends on the CONFIG_GOOGLE_FRAMEBUFFER, which is
standard behavior for all kernel drivers (i.e., drivers depending on
architectural options). Having dependencies in the other direction
creates a circular dependency, which Kconfig doesn't handle.
Yes, okay, I think that makes sense. I thought `DRM_SIMPLEDRM` was
some sort of more general option that the DRM subsystem is enabled,
but I guess it was just the driver that you now replaced with
DRM_COREBOOTDRM, and I understand that you can't have a circular
dependency there.

But
generally speaking, what's the point of having that framebuffer backend
code then? I tried to remove it, but it is supposed to remain. But it
would not serve a purpose other than creating the platform device for
the actual driver. I'm confused about the reason for this design.
I thought the point was just to have a place to put that `if
(sysfs_handles_screen_info()) return -ENODEV;`? (That and that PCI
parent device thing you're doing where I don't really understand the
code enough to know what it's for.)

If you can put that line directly in corebootdrm_probe() instead (and
bind it directly to the coreboot bus device), then that's perfectly
fine by me too, and then you can get rid of framebuffer-coreboot.c. I
just assumed that there was some reason I don't have the expertise to
understand for why you couldn't do that.

My earlier concern here was just about not putting that line into
coreboot_table_populate(), because then you would be preventing the
creation of the sysfs node for that coreboot table entry, while all
other entries that don't have associated drivers are still there (and
that seems inconsistent).

Makes sense. Then let's leave the logic as it is.

Best regards
Thomas



--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)


Reply via email to