Re: [PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers
On Wed, 2022-05-25 at 18:45 +0200, Thomas Zimmermann wrote: > I don't mind adding DRM support for BootX displays, but getting the > necessary test HW with a suitable Linux seems to be laborious. Would > a G4 Powerbook work? Probably not unfortunately... it's going to be tricky. I might sitll have some old BootX-based machines somewhere in storage I could try to dig out, but it might not be worth bothering too much... Cheers, Ben.
Re: [PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers
On Wed, 2022-05-25 at 18:45 +0200, Thomas Zimmermann wrote: > > > The palette handling is useful when using a real Open Firmware > > implementation which tends to boot in 8-bit mode, so without palette > > things will look ... bad. > > > > It's not necessary when using 16/32 bpp framebuffers which is typically > > ... what BootX provides :-) > > Maybe the odd color formats can be tested via qemu. > > I don't mind adding DRM support for BootX displays, but getting the > necessary test HW with a suitable Linux seems to be laborious. Would a > G4 Powerbook work? My point was that it's the non-BootX case that cares about the palette hacks :-) Cheers, Ben.
Re: [PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers
Hi Am 21.05.22 um 04:49 schrieb Benjamin Herrenschmidt: On Thu, 2022-05-19 at 09:27 +0200, Thomas Zimmermann wrote: to build without PCI to see what happens. If you bring any of the "heuristic" and palette support code in, you need PCI. I don't see any reason to take it out. Those old Macs use BootX, right? BootX is not supported ATM, as I don't have the HW to test. Is there an emulator for it? It isn't ? When did it break ? :-) I meant that BootX is not (yet) supported by this new driver. The Linux kernel overall probably supports it. If anyone what's to make patches for BootX, I'd be happy to add them. The offb driver also supports a number of special cases for palette handling. That might be necessary for ofdrm as well. The palette handling is useful when using a real Open Firmware implementation which tends to boot in 8-bit mode, so without palette things will look ... bad. It's not necessary when using 16/32 bpp framebuffers which is typically ... what BootX provides :-) Maybe the odd color formats can be tested via qemu. I don't mind adding DRM support for BootX displays, but getting the necessary test HW with a suitable Linux seems to be laborious. Would a G4 Powerbook work? Best regard Thomas Cheers, Ben. Best regards Thomas Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers
On Sun, 2022-05-22 at 21:35 +0200, Thomas Zimmermann wrote: > > Interesting. Did you find some formats that were not supported ? > > We still don't support XRGB1555. If the native buffer uses this format, > we'd have no conversion helper. In this case, we rely on userspace/fbcon > to use the native format exclusively. (BTW, I asked one of my coworkers > to implement XRGB1555 to make her familiar with DRM. That's why I > haven't sent a patch yet.) > Various old macs do 1555 ... Cheers, Ben.
Re: [PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers
Hi Javier Am 20.05.22 um 08:19 schrieb Javier Martinez Canillas: Hello Thomas, On 5/18/22 20:30, Thomas Zimmermann wrote: +config DRM_OFDRM + tristate "Open Firmware display driver" + depends on DRM && MMU && PPC Shouldn't depend on OF? I mean, is a DRM driver for Open Firmware after all :) I know that the old drivers/video/fbdev/offb.c doesn't, but I think that is a but and should `depends on OF || COMPILE_TEST` I have to look for possible pitfalls, but that seems to makes sense. + +/* + * Helpers for display nodes + */ + +static int display_get_validated_int(struct drm_device *dev, const char *name, uint32_t value) +{ + if (value > INT_MAX) { + drm_err(dev, "invalid framebuffer %s of %u\n", name, value); + return -EINVAL; + } + return (int)value; +} + +static int display_get_validated_int0(struct drm_device *dev, const char *name, uint32_t value) +{ + if (!value) { + drm_err(dev, "invalid framebuffer %s of %u\n", name, value); + return -EINVAL; + } + return display_get_validated_int(dev, name, value); +} + These two helpers are the same that we already have in simpledrm.c, maybe could include a preparatory patch that moves to drivers/gpu/drm/drm_format_helper.c and make them public for drivers to use ? Or maybe even as static inline in include/drm/drm_format_helper.h ? To me it seems that these helpers are so small that they really don't need to be shared. If anything, we could add them directly to the OF module. Something like of_property_read_u32_range() that takes additions min/max values. +static const struct drm_format_info *display_get_validated_format(struct drm_device *dev, + u32 depth) +{ + const struct drm_format_info *info; + u32 format; + + switch (depth) { + case 8: + format = drm_mode_legacy_fb_format(8, 8); + break; + case 15: I think is customary now to add /* fall through */ here to silence GCC warns ? There should be no warnings if multiple case statements are directly next to each other. +} + +static int display_read_u32_of(struct drm_device *dev, struct device_node *of_node, + const char *name, u32 *value) +{ + int ret = of_property_read_u32(of_node, name, value); + + if (ret) + drm_err(dev, "cannot parse framebuffer %s: error %d\n", name, ret); + return ret; +} + [snip] +static u64 display_get_address_of(struct drm_device *dev, struct device_node *of_node) +{ + u32 address; + int ret; + + /* +* Not all devices provide an address property, it's not +* a bug if this fails. The driver will try to find the +* framebuffer base address from the device's memory regions. +*/ + ret = of_property_read_u32(of_node, "address", ); + if (ret) + return OF_BAD_ADDR; + + return address; +} + All these helpers seems to be quite generic and something that other OF drivers could benefit from. Maybe add them to drivers/gpu/drm/drm_of.c ? No point in exporting them AFAICT. They look like the code in simpledrm, but they are specific to this single driver. In this context 'OF' is not OF in general, but the specific generic display of PPC's OF device tree. No other DRM driver should use this. +#if defined(CONFIG_PCI) +static struct pci_dev *display_get_pci_dev_of(struct drm_device *dev, struct device_node *of_node) +{ + const __be32 *vendor_p, *device_p; + u32 vendor, device; + struct pci_dev *pcidev; + + vendor_p = of_get_property(of_node, "vendor-id", NULL); + if (!vendor_p) + return ERR_PTR(-ENODEV); + vendor = be32_to_cpup(vendor_p); + + device_p = of_get_property(of_node, "device-id", NULL); + if (!device_p) + return ERR_PTR(-ENODEV); + device = be32_to_cpup(device_p); + + pcidev = pci_get_device(vendor, device, NULL); + if (!pcidev) + return ERR_PTR(-ENODEV); + + return pcidev; +} +#else +static struct pci_dev *display_get_pci_dev_of(struct drm_device *dev, struct device_node *of_node) +{ + return ERR_PTR(-ENODEV); +} +#endif + Unsure about this one, I don't see other display driver using a "vendor-id" or "device-id" when looking at Documentation/devicetree/bindings/, so guess this one would have to remain in the driver and not in a helper library. But since you have #ifdefery here, maybe would be cleaner to have that stub defined as static inline in include/drm/drm_of.h ? +static struct ofdrm_device *ofdrm_device_of_dev(struct drm_device *dev) +{ + return container_of(dev, struct ofdrm_device, dev); +} + +/* + * OF display settings + */ + This seems like another candidate to move to the include/drm/drm_of.h header. +static struct
Re: [PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers
On Thu, 2022-05-19 at 09:27 +0200, Thomas Zimmermann wrote: > to build without PCI to see what happens. If you bring any of the "heuristic" and palette support code in, you need PCI. I don't see any reason to take it out. > Those old Macs use BootX, right? BootX is not supported ATM, as I don't > have the HW to test. Is there an emulator for it? It isn't ? When did it break ? :-) > If anyone what's to make patches for BootX, I'd be happy to add them. > The offb driver also supports a number of special cases for palette > handling. That might be necessary for ofdrm as well. The palette handling is useful when using a real Open Firmware implementation which tends to boot in 8-bit mode, so without palette things will look ... bad. It's not necessary when using 16/32 bpp framebuffers which is typically ... what BootX provides :-) Cheers, Ben. > Best regards > Thomas > > > Gr{oetje,eeting}s, > > > > Geert > > > > -- > > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > > ge...@linux-m68k.org > > > > In personal conversations with technical people, I call myself a hacker. But > > when I'm talking to journalists I just say "programmer" or something like > > that. > > -- Linus Torvalds > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Ivo Totev
Re: [PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers
On Thu, 2022-05-19 at 09:11 +0200, Geert Uytterhoeven wrote: > Hi Michal, > > > > On Wed, May 18, 2022 at 8:54 PM Michal Suchánek > wrote: > > > On Wed, May 18, 2022 at 08:30:06PM +0200, Thomas Zimmermann wrote: > > > --- a/drivers/gpu/drm/tiny/Kconfig > > > +++ b/drivers/gpu/drm/tiny/Kconfig > > > @@ -51,6 +51,18 @@ config DRM_GM12U320 > > > This is a KMS driver for projectors which use the > > > GM12U320 chipset > > > for video transfer over USB2/3, such as the Acer C120 > > > mini projector. > > > +config DRM_OFDRM > > > + tristate "Open Firmware display driver" > > > + depends on DRM && MMU && PPC > > Does this build with !PCI? > > The driver uses some PCI functions, so it might possibly break with > > randconfig. I don't think there are practical !PCI PPC > > configurations. > > > IIRC, the first PowerMacs didn't have PCI. They also don't have OF and we never supported them upstream :-) Cheers, Ben.
Re: [PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers
Hello Thomas, On 5/18/22 20:30, Thomas Zimmermann wrote: > > +config DRM_OFDRM > + tristate "Open Firmware display driver" > + depends on DRM && MMU && PPC Shouldn't depend on OF? I mean, is a DRM driver for Open Firmware after all :) I know that the old drivers/video/fbdev/offb.c doesn't, but I think that is a but and should `depends on OF || COMPILE_TEST` > + > +/* > + * Helpers for display nodes > + */ > + > +static int display_get_validated_int(struct drm_device *dev, const char > *name, uint32_t value) > +{ > + if (value > INT_MAX) { > + drm_err(dev, "invalid framebuffer %s of %u\n", name, value); > + return -EINVAL; > + } > + return (int)value; > +} > + > +static int display_get_validated_int0(struct drm_device *dev, const char > *name, uint32_t value) > +{ > + if (!value) { > + drm_err(dev, "invalid framebuffer %s of %u\n", name, value); > + return -EINVAL; > + } > + return display_get_validated_int(dev, name, value); > +} > + These two helpers are the same that we already have in simpledrm.c, maybe could include a preparatory patch that moves to drivers/gpu/drm/drm_format_helper.c and make them public for drivers to use ? Or maybe even as static inline in include/drm/drm_format_helper.h ? > +static const struct drm_format_info *display_get_validated_format(struct > drm_device *dev, > + u32 depth) > +{ > + const struct drm_format_info *info; > + u32 format; > + > + switch (depth) { > + case 8: > + format = drm_mode_legacy_fb_format(8, 8); > + break; > + case 15: I think is customary now to add /* fall through */ here to silence GCC warns ? > +} > + > +static int display_read_u32_of(struct drm_device *dev, struct device_node > *of_node, > +const char *name, u32 *value) > +{ > + int ret = of_property_read_u32(of_node, name, value); > + > + if (ret) > + drm_err(dev, "cannot parse framebuffer %s: error %d\n", name, > ret); > + return ret; > +} > + [snip] > +static u64 display_get_address_of(struct drm_device *dev, struct device_node > *of_node) > +{ > + u32 address; > + int ret; > + > + /* > + * Not all devices provide an address property, it's not > + * a bug if this fails. The driver will try to find the > + * framebuffer base address from the device's memory regions. > + */ > + ret = of_property_read_u32(of_node, "address", ); > + if (ret) > + return OF_BAD_ADDR; > + > + return address; > +} > + All these helpers seems to be quite generic and something that other OF drivers could benefit from. Maybe add them to drivers/gpu/drm/drm_of.c ? > +#if defined(CONFIG_PCI) > +static struct pci_dev *display_get_pci_dev_of(struct drm_device *dev, struct > device_node *of_node) > +{ > + const __be32 *vendor_p, *device_p; > + u32 vendor, device; > + struct pci_dev *pcidev; > + > + vendor_p = of_get_property(of_node, "vendor-id", NULL); > + if (!vendor_p) > + return ERR_PTR(-ENODEV); > + vendor = be32_to_cpup(vendor_p); > + > + device_p = of_get_property(of_node, "device-id", NULL); > + if (!device_p) > + return ERR_PTR(-ENODEV); > + device = be32_to_cpup(device_p); > + > + pcidev = pci_get_device(vendor, device, NULL); > + if (!pcidev) > + return ERR_PTR(-ENODEV); > + > + return pcidev; > +} > +#else > +static struct pci_dev *display_get_pci_dev_of(struct drm_device *dev, struct > device_node *of_node) > +{ > + return ERR_PTR(-ENODEV); > +} > +#endif > + Unsure about this one, I don't see other display driver using a "vendor-id" or "device-id" when looking at Documentation/devicetree/bindings/, so guess this one would have to remain in the driver and not in a helper library. But since you have #ifdefery here, maybe would be cleaner to have that stub defined as static inline in include/drm/drm_of.h ? > +static struct ofdrm_device *ofdrm_device_of_dev(struct drm_device *dev) > +{ > + return container_of(dev, struct ofdrm_device, dev); > +} > + > +/* > + * OF display settings > + */ > + This seems like another candidate to move to the include/drm/drm_of.h header. > +static struct drm_display_mode ofdrm_mode(unsigned int width, unsigned int > height) > +{ > + struct drm_display_mode mode = { OFDRM_MODE(width, height) }; > + > + mode.clock = mode.hdisplay * mode.vdisplay * 60 / 1000 /* kHz */; Maybe a comment here about the clock value chosen ? > + drm_mode_set_name(); > + > + return mode; > +} > + [snip] > + > + /* > + * Never use pcim_ or other managed helpers on the returned PCI > + * device. Otherwise, probing the native driver will fail for > + * resource conflicts. PCI-device management has to be tied to > + * the lifetime of the platform device until the
Re: [PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers
Hi Am 18.05.22 um 23:11 schrieb Mark Cave-Ayland: On 18/05/2022 19:30, Thomas Zimmermann wrote: Open Firmware provides basic display output via the 'display' node. DT platform code already provides a device that represents the node's framebuffer. Add a DRM driver for the device. The display mode and color format is pre-initialized by the system's firmware. Runtime modesetting via DRM is not possible. The display is useful during early boot stages or as error fallback. Similar functionality is already provided by fbdev's offb driver, which is insufficient for modern userspace. The old driver includes support for BootX device tree, which can be found on old 32-bit PowerPC Macintosh systems. If these are still in use, the functionality can be added to ofdrm or implemented in a new driver. As with simepldrm, the fbdev driver cannot be selected is ofdrm is already enabled. Two noteable points about the driver: * Reading the framebuffer aperture from the device tree is not reliable on all systems. Ofdrm takes the heuristics and a comment from offb to pick the correct range. * No resource management may be tied to the underlying PCI device. Otherwise the handover to the native driver will fail with a resource conflict. PCI management is therefore done as part of the platform device's cleanup. The driver has been tested on qemu's ppc64le emulation. The device hand-over has been tested with bochs. Thanks for working on this! Have you tried it on qemu-system-sparc and qemu-system-sparc64 at all? At least under QEMU I'd expect it to work for these platforms too, unless there is a particular dependency on PCI. A couple of comments inline below: I haven't tried Sparc, as the old offb driver only supports PPC. I was already wondering why Sparc isn't supported. I wouldn't mind if we can add it. Signed-off-by: Thomas Zimmermann --- MAINTAINERS | 1 + drivers/gpu/drm/tiny/Kconfig | 12 + drivers/gpu/drm/tiny/Makefile | 1 + drivers/gpu/drm/tiny/ofdrm.c | 748 ++ drivers/video/fbdev/Kconfig | 1 + 5 files changed, 763 insertions(+) create mode 100644 drivers/gpu/drm/tiny/ofdrm.c diff --git a/MAINTAINERS b/MAINTAINERS index 43d833273ae9..090cbe1aa5e3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6395,6 +6395,7 @@ L: dri-de...@lists.freedesktop.org S: Maintained T: git git://anongit.freedesktop.org/drm/drm-misc F: drivers/gpu/drm/drm_aperture.c +F: drivers/gpu/drm/tiny/ofdrm.c F: drivers/gpu/drm/tiny/simpledrm.c F: include/drm/drm_aperture.h diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig index 627d637a1e7e..0bc54af42e7f 100644 --- a/drivers/gpu/drm/tiny/Kconfig +++ b/drivers/gpu/drm/tiny/Kconfig @@ -51,6 +51,18 @@ config DRM_GM12U320 This is a KMS driver for projectors which use the GM12U320 chipset for video transfer over USB2/3, such as the Acer C120 mini projector. +config DRM_OFDRM + tristate "Open Firmware display driver" + depends on DRM && MMU && PPC + select DRM_GEM_SHMEM_HELPER + select DRM_KMS_HELPER + help + DRM driver for Open Firmware framebuffers. + + This driver assumes that the display hardware has been initialized + by the Open Firmware before the kernel boots. Scanout buffer, size, + and display format must be provided via device tree. + config DRM_PANEL_MIPI_DBI tristate "DRM support for MIPI DBI compatible panels" depends on DRM && SPI diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile index 1d9d6227e7ab..76dde89a044b 100644 --- a/drivers/gpu/drm/tiny/Makefile +++ b/drivers/gpu/drm/tiny/Makefile @@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o obj-$(CONFIG_DRM_BOCHS) += bochs.o obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o obj-$(CONFIG_DRM_GM12U320) += gm12u320.o +obj-$(CONFIG_DRM_OFDRM) += ofdrm.o obj-$(CONFIG_DRM_PANEL_MIPI_DBI) += panel-mipi-dbi.o obj-$(CONFIG_DRM_SIMPLEDRM) += simpledrm.o obj-$(CONFIG_TINYDRM_HX8357D) += hx8357d.o diff --git a/drivers/gpu/drm/tiny/ofdrm.c b/drivers/gpu/drm/tiny/ofdrm.c new file mode 100644 index ..aca715b36179 --- /dev/null +++ b/drivers/gpu/drm/tiny/ofdrm.c @@ -0,0 +1,748 @@ +// SPDX-License-Identifier: GPL-2.0-only + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + + +#define DRIVER_NAME "ofdrm" +#define DRIVER_DESC "DRM driver for OF platform devices" +#define DRIVER_DATE "20220501" +#define DRIVER_MAJOR 1 +#define DRIVER_MINOR 0 + +/* + * Assume a monitor resolution of 96 dpi to + * get a somewhat reasonable screen size. + */ +#define RES_MM(d) \ + (((d) * 254ul) / (96ul * 10ul)) + +#define OFDRM_MODE(hd, vd) \ + DRM_SIMPLE_MODE(hd, vd, RES_MM(hd), RES_MM(vd)) + +/* + *
Re: [PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers
Hi Am 19.05.22 um 09:11 schrieb Geert Uytterhoeven: Hi Michal, On Wed, May 18, 2022 at 8:54 PM Michal Suchánek wrote: On Wed, May 18, 2022 at 08:30:06PM +0200, Thomas Zimmermann wrote: --- a/drivers/gpu/drm/tiny/Kconfig +++ b/drivers/gpu/drm/tiny/Kconfig @@ -51,6 +51,18 @@ config DRM_GM12U320 This is a KMS driver for projectors which use the GM12U320 chipset for video transfer over USB2/3, such as the Acer C120 mini projector. +config DRM_OFDRM + tristate "Open Firmware display driver" + depends on DRM && MMU && PPC Does this build with !PCI? The driver uses some PCI functions, so it might possibly break with randconfig. I don't think there are practical !PCI PPC configurations. IIRC, the first PowerMacs didn't have PCI. I'll try to build without PCI to see what happens. Those old Macs use BootX, right? BootX is not supported ATM, as I don't have the HW to test. Is there an emulator for it? If anyone what's to make patches for BootX, I'd be happy to add them. The offb driver also supports a number of special cases for palette handling. That might be necessary for ofdrm as well. Best regards Thomas Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers
Hi Michal, On Wed, May 18, 2022 at 8:54 PM Michal Suchánek wrote: > On Wed, May 18, 2022 at 08:30:06PM +0200, Thomas Zimmermann wrote: > > --- a/drivers/gpu/drm/tiny/Kconfig > > +++ b/drivers/gpu/drm/tiny/Kconfig > > @@ -51,6 +51,18 @@ config DRM_GM12U320 > >This is a KMS driver for projectors which use the GM12U320 chipset > >for video transfer over USB2/3, such as the Acer C120 mini projector. > > > > +config DRM_OFDRM > > + tristate "Open Firmware display driver" > > + depends on DRM && MMU && PPC > > Does this build with !PCI? > > The driver uses some PCI functions, so it might possibly break with > randconfig. I don't think there are practical !PCI PPC configurations. IIRC, the first PowerMacs didn't have PCI. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers
On Wed, May 18, 2022 at 10:11:03PM +0100, Mark Cave-Ayland wrote: > On 18/05/2022 19:30, Thomas Zimmermann wrote: > > > Open Firmware provides basic display output via the 'display' node. > > DT platform code already provides a device that represents the node's > > framebuffer. Add a DRM driver for the device. The display mode and > > color format is pre-initialized by the system's firmware. Runtime > > modesetting via DRM is not possible. The display is useful during > > early boot stages or as error fallback. > > > > Similar functionality is already provided by fbdev's offb driver, > > which is insufficient for modern userspace. The old driver includes > > support for BootX device tree, which can be found on old 32-bit > > PowerPC Macintosh systems. If these are still in use, the > > functionality can be added to ofdrm or implemented in a new > > driver. As with simepldrm, the fbdev driver cannot be selected is > > ofdrm is already enabled. > > > > Two noteable points about the driver: > > > > * Reading the framebuffer aperture from the device tree is not > > reliable on all systems. Ofdrm takes the heuristics and a comment > > from offb to pick the correct range. > > > > * No resource management may be tied to the underlying PCI device. > > Otherwise the handover to the native driver will fail with a resource > > conflict. PCI management is therefore done as part of the platform > > device's cleanup. > > > > The driver has been tested on qemu's ppc64le emulation. The device > > hand-over has been tested with bochs. > > Thanks for working on this! Have you tried it on qemu-system-sparc and > qemu-system-sparc64 at all? At least under QEMU I'd expect it to work for > these platforms too, unless there is a particular dependency on PCI. A There is an implicit dependency on PCI, and it won't work because it depends on PPC: depends on DRM && MMU && PPC this is what the offb has, too. I am wondering what is the driver for OF based framebuffer on sparc and arm but offb clearly isn't with its dependency on PPC. Thanks Michal > couple of comments inline below: > > > Signed-off-by: Thomas Zimmermann > > --- > > MAINTAINERS | 1 + > > drivers/gpu/drm/tiny/Kconfig | 12 + > > drivers/gpu/drm/tiny/Makefile | 1 + > > drivers/gpu/drm/tiny/ofdrm.c | 748 ++ > > drivers/video/fbdev/Kconfig | 1 + > > 5 files changed, 763 insertions(+) > > create mode 100644 drivers/gpu/drm/tiny/ofdrm.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 43d833273ae9..090cbe1aa5e3 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -6395,6 +6395,7 @@ L:dri-de...@lists.freedesktop.org > > S:Maintained > > T:git git://anongit.freedesktop.org/drm/drm-misc > > F:drivers/gpu/drm/drm_aperture.c > > +F: drivers/gpu/drm/tiny/ofdrm.c > > F:drivers/gpu/drm/tiny/simpledrm.c > > F:include/drm/drm_aperture.h > > diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig > > index 627d637a1e7e..0bc54af42e7f 100644 > > --- a/drivers/gpu/drm/tiny/Kconfig > > +++ b/drivers/gpu/drm/tiny/Kconfig > > @@ -51,6 +51,18 @@ config DRM_GM12U320 > > This is a KMS driver for projectors which use the GM12U320 chipset > > for video transfer over USB2/3, such as the Acer C120 mini projector. > > +config DRM_OFDRM > > + tristate "Open Firmware display driver" > > + depends on DRM && MMU && PPC > > + select DRM_GEM_SHMEM_HELPER > > + select DRM_KMS_HELPER > > + help > > + DRM driver for Open Firmware framebuffers. > > + > > + This driver assumes that the display hardware has been initialized > > + by the Open Firmware before the kernel boots. Scanout buffer, size, > > + and display format must be provided via device tree. > > + > > config DRM_PANEL_MIPI_DBI > > tristate "DRM support for MIPI DBI compatible panels" > > depends on DRM && SPI > > diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile > > index 1d9d6227e7ab..76dde89a044b 100644 > > --- a/drivers/gpu/drm/tiny/Makefile > > +++ b/drivers/gpu/drm/tiny/Makefile > > @@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_ARCPGU)+= arcpgu.o > > obj-$(CONFIG_DRM_BOCHS) += bochs.o > > obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o > > obj-$(CONFIG_DRM_GM12U320)+= gm12u320.o > > +obj-$(CONFIG_DRM_OFDRM)+= ofdrm.o > > obj-$(CONFIG_DRM_PANEL_MIPI_DBI) += panel-mipi-dbi.o > > obj-$(CONFIG_DRM_SIMPLEDRM) += simpledrm.o > > obj-$(CONFIG_TINYDRM_HX8357D) += hx8357d.o > > diff --git a/drivers/gpu/drm/tiny/ofdrm.c b/drivers/gpu/drm/tiny/ofdrm.c > > new file mode 100644 > > index ..aca715b36179 > > --- /dev/null > > +++ b/drivers/gpu/drm/tiny/ofdrm.c > > @@ -0,0 +1,748 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > + > > +#include > > +#include > > + > > +#include > > +#include > >
Re: [PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers
On 18/05/2022 19:30, Thomas Zimmermann wrote: Open Firmware provides basic display output via the 'display' node. DT platform code already provides a device that represents the node's framebuffer. Add a DRM driver for the device. The display mode and color format is pre-initialized by the system's firmware. Runtime modesetting via DRM is not possible. The display is useful during early boot stages or as error fallback. Similar functionality is already provided by fbdev's offb driver, which is insufficient for modern userspace. The old driver includes support for BootX device tree, which can be found on old 32-bit PowerPC Macintosh systems. If these are still in use, the functionality can be added to ofdrm or implemented in a new driver. As with simepldrm, the fbdev driver cannot be selected is ofdrm is already enabled. Two noteable points about the driver: * Reading the framebuffer aperture from the device tree is not reliable on all systems. Ofdrm takes the heuristics and a comment from offb to pick the correct range. * No resource management may be tied to the underlying PCI device. Otherwise the handover to the native driver will fail with a resource conflict. PCI management is therefore done as part of the platform device's cleanup. The driver has been tested on qemu's ppc64le emulation. The device hand-over has been tested with bochs. Thanks for working on this! Have you tried it on qemu-system-sparc and qemu-system-sparc64 at all? At least under QEMU I'd expect it to work for these platforms too, unless there is a particular dependency on PCI. A couple of comments inline below: Signed-off-by: Thomas Zimmermann --- MAINTAINERS | 1 + drivers/gpu/drm/tiny/Kconfig | 12 + drivers/gpu/drm/tiny/Makefile | 1 + drivers/gpu/drm/tiny/ofdrm.c | 748 ++ drivers/video/fbdev/Kconfig | 1 + 5 files changed, 763 insertions(+) create mode 100644 drivers/gpu/drm/tiny/ofdrm.c diff --git a/MAINTAINERS b/MAINTAINERS index 43d833273ae9..090cbe1aa5e3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6395,6 +6395,7 @@ L:dri-de...@lists.freedesktop.org S:Maintained T:git git://anongit.freedesktop.org/drm/drm-misc F:drivers/gpu/drm/drm_aperture.c +F: drivers/gpu/drm/tiny/ofdrm.c F:drivers/gpu/drm/tiny/simpledrm.c F:include/drm/drm_aperture.h diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig index 627d637a1e7e..0bc54af42e7f 100644 --- a/drivers/gpu/drm/tiny/Kconfig +++ b/drivers/gpu/drm/tiny/Kconfig @@ -51,6 +51,18 @@ config DRM_GM12U320 This is a KMS driver for projectors which use the GM12U320 chipset for video transfer over USB2/3, such as the Acer C120 mini projector. +config DRM_OFDRM + tristate "Open Firmware display driver" + depends on DRM && MMU && PPC + select DRM_GEM_SHMEM_HELPER + select DRM_KMS_HELPER + help + DRM driver for Open Firmware framebuffers. + + This driver assumes that the display hardware has been initialized + by the Open Firmware before the kernel boots. Scanout buffer, size, + and display format must be provided via device tree. + config DRM_PANEL_MIPI_DBI tristate "DRM support for MIPI DBI compatible panels" depends on DRM && SPI diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile index 1d9d6227e7ab..76dde89a044b 100644 --- a/drivers/gpu/drm/tiny/Makefile +++ b/drivers/gpu/drm/tiny/Makefile @@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_ARCPGU)+= arcpgu.o obj-$(CONFIG_DRM_BOCHS) += bochs.o obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o obj-$(CONFIG_DRM_GM12U320)+= gm12u320.o +obj-$(CONFIG_DRM_OFDRM)+= ofdrm.o obj-$(CONFIG_DRM_PANEL_MIPI_DBI) += panel-mipi-dbi.o obj-$(CONFIG_DRM_SIMPLEDRM) += simpledrm.o obj-$(CONFIG_TINYDRM_HX8357D) += hx8357d.o diff --git a/drivers/gpu/drm/tiny/ofdrm.c b/drivers/gpu/drm/tiny/ofdrm.c new file mode 100644 index ..aca715b36179 --- /dev/null +++ b/drivers/gpu/drm/tiny/ofdrm.c @@ -0,0 +1,748 @@ +// SPDX-License-Identifier: GPL-2.0-only + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + + +#define DRIVER_NAME"ofdrm" +#define DRIVER_DESC"DRM driver for OF platform devices" +#define DRIVER_DATE"20220501" +#define DRIVER_MAJOR 1 +#define DRIVER_MINOR 0 + +/* + * Assume a monitor resolution of 96 dpi to + * get a somewhat reasonable screen size. + */ +#define RES_MM(d) \ + (((d) * 254ul) / (96ul * 10ul)) + +#define OFDRM_MODE(hd, vd) \ + DRM_SIMPLE_MODE(hd, vd, RES_MM(hd), RES_MM(vd)) + +/* + * Helpers for display nodes + */ + +static int display_get_validated_int(struct drm_device *dev, const char *name,
Re: [PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers
Hello, On Wed, May 18, 2022 at 08:30:06PM +0200, Thomas Zimmermann wrote: > Open Firmware provides basic display output via the 'display' node. > DT platform code already provides a device that represents the node's > framebuffer. Add a DRM driver for the device. The display mode and > color format is pre-initialized by the system's firmware. Runtime > modesetting via DRM is not possible. The display is useful during > early boot stages or as error fallback. > > Similar functionality is already provided by fbdev's offb driver, > which is insufficient for modern userspace. The old driver includes > support for BootX device tree, which can be found on old 32-bit > PowerPC Macintosh systems. If these are still in use, the > functionality can be added to ofdrm or implemented in a new > driver. As with simepldrm, the fbdev driver cannot be selected is > ofdrm is already enabled. > > Two noteable points about the driver: > > * Reading the framebuffer aperture from the device tree is not > reliable on all systems. Ofdrm takes the heuristics and a comment > from offb to pick the correct range. > > * No resource management may be tied to the underlying PCI device. > Otherwise the handover to the native driver will fail with a resource > conflict. PCI management is therefore done as part of the platform > device's cleanup. > > The driver has been tested on qemu's ppc64le emulation. The device > hand-over has been tested with bochs. > > Signed-off-by: Thomas Zimmermann > --- > MAINTAINERS | 1 + > drivers/gpu/drm/tiny/Kconfig | 12 + > drivers/gpu/drm/tiny/Makefile | 1 + > drivers/gpu/drm/tiny/ofdrm.c | 748 ++ > drivers/video/fbdev/Kconfig | 1 + > 5 files changed, 763 insertions(+) > create mode 100644 drivers/gpu/drm/tiny/ofdrm.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 43d833273ae9..090cbe1aa5e3 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6395,6 +6395,7 @@ L: dri-de...@lists.freedesktop.org > S: Maintained > T: git git://anongit.freedesktop.org/drm/drm-misc > F: drivers/gpu/drm/drm_aperture.c > +F: drivers/gpu/drm/tiny/ofdrm.c > F: drivers/gpu/drm/tiny/simpledrm.c > F: include/drm/drm_aperture.h > > diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig > index 627d637a1e7e..0bc54af42e7f 100644 > --- a/drivers/gpu/drm/tiny/Kconfig > +++ b/drivers/gpu/drm/tiny/Kconfig > @@ -51,6 +51,18 @@ config DRM_GM12U320 >This is a KMS driver for projectors which use the GM12U320 chipset >for video transfer over USB2/3, such as the Acer C120 mini projector. > > +config DRM_OFDRM > + tristate "Open Firmware display driver" > + depends on DRM && MMU && PPC Does this build with !PCI? The driver uses some PCI functions, so it might possibly break with randconfig. I don't think there are practical !PCI PPC configurations. Thanks Michal