Hi Am 29.06.20 um 11:27 schrieb Daniel Vetter: > On Thu, Jun 25, 2020 at 02:00:10PM +0200, Thomas Zimmermann wrote: >> Platform devices might operate on firmware framebuffers, such as VESA or >> EFI. Before a native driver for the graphics hardware can take over the >> device, it has to remove any platform driver that operates on the firmware >> framebuffer. Platform helpers provide the infrastructure for platform >> drivers to acquire firmware framebuffers, and for native drivers to remove >> them lateron. >> >> It works similar to the related fbdev mechanism. During initialization, the >> platform driver acquires the firmware framebuffer's I/O memory and provides >> a callback to be removed. The native driver later uses this inforamtion to >> remove any platform driver for it's framebuffer I/O memory. >> >> The platform helper's removal code is integrated into the existing code for >> removing conflicting fraembuffers, so native drivers use it automatically. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx> > > I have some ideas for how to do this a notch cleaner in the next patch. > Maybe best to discuss the actual implmenentation stuff there. > > Aside from that usual nits: > - kerneldoc for these please, pulled into drm-kms.rst. Any specific reason for drm-kms? The aperture helpers are used to manage ownership of memory and most drivers begin with drm_fb_helper_remove_conflicting_framebuffers(). It seems more approprite to put this into drm-internals as part of the driver initialization. Best regards Thomas > - naming isn't super ocd with drm_platform.c but that prefix not used, but > I also don't have better ideas. > - I think the functions from drm_fb_helper.h for removing other > framebuffers should be moved here, and function name prefix adjusted > acoordingly > > I'm wondering about the locking and deadlock potential here, is lockdep > all happy with this? > > Cheers, Daniel > >> --- >> drivers/gpu/drm/Kconfig | 6 ++ >> drivers/gpu/drm/Makefile | 1 + >> drivers/gpu/drm/drm_platform.c | 118 +++++++++++++++++++++++++++++++++ >> include/drm/drm_fb_helper.h | 18 ++++- >> include/drm/drm_platform.h | 42 ++++++++++++ >> 5 files changed, 184 insertions(+), 1 deletion(-) >> create mode 100644 drivers/gpu/drm/drm_platform.c >> create mode 100644 include/drm/drm_platform.h >> >> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >> index c4fd57d8b717..e9d6892f9d38 100644 >> --- a/drivers/gpu/drm/Kconfig >> +++ b/drivers/gpu/drm/Kconfig >> @@ -229,6 +229,12 @@ config DRM_SCHED >> tristate >> depends on DRM >> >> +config DRM_PLATFORM_HELPER >> + bool >> + depends on DRM >> + help >> + Helpers for DRM platform devices >> + >> source "drivers/gpu/drm/i2c/Kconfig" >> >> source "drivers/gpu/drm/arm/Kconfig" >> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile >> index 2c0e5a7e5953..8ceb21d0770a 100644 >> --- a/drivers/gpu/drm/Makefile >> +++ b/drivers/gpu/drm/Makefile >> @@ -32,6 +32,7 @@ drm-$(CONFIG_AGP) += drm_agpsupport.o >> drm-$(CONFIG_PCI) += drm_pci.o >> drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o >> drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o >> +drm-$(CONFIG_DRM_PLATFORM_HELPER) += drm_platform.o >> >> drm_vram_helper-y := drm_gem_vram_helper.o >> obj-$(CONFIG_DRM_VRAM_HELPER) += drm_vram_helper.o >> diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c >> new file mode 100644 >> index 000000000000..09a2f2a31aa5 >> --- /dev/null >> +++ b/drivers/gpu/drm/drm_platform.c >> @@ -0,0 +1,118 @@ >> +// SPDX-License-Identifier: GPL-2.0 OR MIT >> + >> +#include <linux/mutex.h> >> +#include <linux/slab.h> >> + >> +#include <drm/drm_drv.h> >> +#include <drm/drm_managed.h> >> +#include <drm/drm_platform.h> >> + >> +static LIST_HEAD(drm_apertures); >> + >> +static DEFINE_MUTEX(drm_apertures_lock); >> + >> +static bool overlap(resource_size_t base1, resource_size_t end1, >> + resource_size_t base2, resource_size_t end2) >> +{ >> + return (base1 < end2) && (end1 > base2); >> +} >> + >> +static struct drm_aperture * >> +drm_aperture_acquire(struct drm_device *dev, >> + resource_size_t base, resource_size_t size, >> + const struct drm_aperture_funcs *funcs) >> +{ >> + size_t end = base + size; >> + struct list_head *pos; >> + struct drm_aperture *ap; >> + >> + mutex_lock(&drm_apertures_lock); >> + >> + list_for_each(pos, &drm_apertures) { >> + ap = container_of(pos, struct drm_aperture, lh); >> + if (overlap(base, end, ap->base, ap->base + ap->size)) >> + return ERR_PTR(-EBUSY); >> + } >> + >> + ap = drmm_kzalloc(dev, sizeof(*ap), GFP_KERNEL); >> + if (!ap) >> + return ERR_PTR(-ENOMEM); >> + >> + ap->dev = dev; >> + ap->base = base; >> + ap->size = size; >> + ap->funcs = funcs; >> + INIT_LIST_HEAD(&ap->lh); >> + >> + list_add(&ap->lh, &drm_apertures); >> + >> + mutex_unlock(&drm_apertures_lock); >> + >> + return ap; >> +} >> + >> +static void drm_aperture_release(struct drm_aperture *ap) >> +{ >> + bool kicked_out = ap->kicked_out; >> + >> + if (!kicked_out) >> + mutex_lock(&drm_apertures_lock); >> + >> + list_del(&ap->lh); >> + if (ap->funcs->release) >> + ap->funcs->release(ap); >> + >> + if (!kicked_out) >> + mutex_unlock(&drm_apertures_lock); >> +} >> + >> +static void drm_aperture_acquire_release(struct drm_device *dev, void *ptr) >> +{ >> + struct drm_aperture *ap = ptr; >> + >> + drm_aperture_release(ap); >> +} >> + >> +struct drm_aperture * >> +drmm_aperture_acquire(struct drm_device *dev, >> + resource_size_t base, resource_size_t size, >> + const struct drm_aperture_funcs *funcs) >> +{ >> + struct drm_aperture *ap; >> + int ret; >> + >> + ap = drm_aperture_acquire(dev, base, size, funcs); >> + if (IS_ERR(ap)) >> + return ap; >> + ret = drmm_add_action_or_reset(dev, drm_aperture_acquire_release, ap); >> + if (ret) >> + return ERR_PTR(ret); >> + >> + return ap; >> +} >> +EXPORT_SYMBOL(drmm_aperture_acquire); >> + >> +void drm_kickout_apertures_at(resource_size_t base, resource_size_t size) >> +{ >> + resource_size_t end = base + size; >> + struct list_head *pos, *n; >> + >> + mutex_lock(&drm_apertures_lock); >> + >> + list_for_each_safe(pos, n, &drm_apertures) { >> + struct drm_aperture *ap = >> + container_of(pos, struct drm_aperture, lh); >> + >> + if (!overlap(base, end, ap->base, ap->base + ap->size)) >> + continue; >> + >> + ap->kicked_out = true; >> + if (ap->funcs->kickout) >> + ap->funcs->kickout(ap); >> + else >> + drm_dev_put(ap->dev); >> + } >> + >> + mutex_unlock(&drm_apertures_lock); >> +} >> +EXPORT_SYMBOL(drm_kickout_apertures_at); >> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h >> index 306aa3a60be9..a919b78b1961 100644 >> --- a/include/drm/drm_fb_helper.h >> +++ b/include/drm/drm_fb_helper.h >> @@ -35,7 +35,9 @@ struct drm_fb_helper; >> #include <drm/drm_client.h> >> #include <drm/drm_crtc.h> >> #include <drm/drm_device.h> >> +#include <drm/drm_platform.h> >> #include <linux/kgdb.h> >> +#include <linux/pci.h> >> #include <linux/vgaarb.h> >> >> enum mode_set_atomic { >> @@ -465,6 +467,11 @@ static inline int >> drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a, >> const char *name, bool primary) >> { >> + int i; >> + >> + for (i = 0; i < a->count; ++i) >> + drm_kickout_apertures_at(a->ranges[i].base, a->ranges[i].size); >> + >> #if IS_REACHABLE(CONFIG_FB) >> return remove_conflicting_framebuffers(a, name, primary); >> #else >> @@ -487,7 +494,16 @@ static inline int >> drm_fb_helper_remove_conflicting_pci_framebuffers(struct pci_dev *pdev, >> const char *name) >> { >> - int ret = 0; >> + resource_size_t base, size; >> + int bar, ret = 0; >> + >> + for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) { >> + if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM)) >> + continue; >> + base = pci_resource_start(pdev, bar); >> + size = pci_resource_len(pdev, bar); >> + drm_kickout_apertures_at(base, size); >> + } >> >> /* >> * WARNING: Apparently we must kick fbdev drivers before vgacon, >> diff --git a/include/drm/drm_platform.h b/include/drm/drm_platform.h >> new file mode 100644 >> index 000000000000..475e88ee1fbd >> --- /dev/null >> +++ b/include/drm/drm_platform.h >> @@ -0,0 +1,42 @@ >> +// SPDX-License-Identifier: GPL-2.0 OR MIT >> + >> +#ifndef _DRM_PLATFORM_H_ >> +#define _DRM_PLATFORM_H_ >> + >> +#include <linux/list.h> >> +#include <linux/types.h> >> + >> +struct drm_aperture; >> +struct drm_device; >> + >> +struct drm_aperture_funcs { >> + void (*kickout)(struct drm_aperture *ap); >> + void (*release)(struct drm_aperture *ap); >> +}; >> + >> +struct drm_aperture { >> + struct drm_device *dev; >> + resource_size_t base; >> + resource_size_t size; >> + >> + const struct drm_aperture_funcs *funcs; >> + >> + struct list_head lh; >> + bool kicked_out; >> +}; >> + >> +struct drm_aperture * >> +drmm_aperture_acquire(struct drm_device *dev, >> + resource_size_t base, resource_size_t size, >> + const struct drm_aperture_funcs *funcs); >> + >> +#if defined (CONFIG_DRM_PLATFORM_HELPER) >> +void drm_kickout_apertures_at(resource_size_t base, resource_size_t size); >> +#else >> +static inline void >> +drm_kickout_apertures_at(resource_size_t base, resource_size_t size) >> +{ >> +} >> +#endif >> + >> +#endif >> -- >> 2.27.0 >> > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel