| Message ID | 20200625120011.16168-9-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Support simple-framebuffer devices and firmware fbs | expand |
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@suse.de> 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. - 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 >
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@suse.de> > --- > 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); One technical thing here, but that'll be even more clear once the next patch is using the driver core detach function: This should be devm_ not drmm_ (both here and below) because it's tied to the lifetime of the physical device and the driver binding. Once the driver is unbound, even if drm_device keeps surviving because userspace is still holding references, there's no point in trying to unbind it again. So auto-cleanup using devm_ is the right thing here imo. And with the change to the driver model unbind you will have a struct device * pointer, so this all works out. -Daniel > + 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 >
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@suse.de> > > 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. > - naming isn't super ocd with drm_platform.c but that prefix not used, but > I also don't have better ideas. I was never really happy with the naming either. Maybe call it aperture helpers with drm_aperture_ and CONFIG_DRM_APERTURE_HELPER? > - 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? I haven't seen any complains, but I'll double check. Best regards Thomas > > 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 >> >
On Mon, Sep 28, 2020 at 10:40 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > 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@suse.de> > > > > 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. > > - naming isn't super ocd with drm_platform.c but that prefix not used, but > > I also don't have better ideas. > > I was never really happy with the naming either. Maybe call it aperture > helpers with drm_aperture_ and CONFIG_DRM_APERTURE_HELPER? +1 > > - 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? > > I haven't seen any complains, but I'll double check. Might be worth it to cc Greg on this, since the device driver model has a lot of corner cases to make sure it doesn't get stuck here with hiararchical drivers/device getting removed while something else is probing them at the same time. -Daniel > Best regards > Thomas > > > > > 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 >
Hi Am 28.09.20 um 10:50 schrieb Daniel Vetter: > On Mon, Sep 28, 2020 at 10:40 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: >> >> 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@suse.de> >>> >>> 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. >>> - naming isn't super ocd with drm_platform.c but that prefix not used, but >>> I also don't have better ideas. >> >> I was never really happy with the naming either. Maybe call it aperture >> helpers with drm_aperture_ and CONFIG_DRM_APERTURE_HELPER? > > +1 > >>> - 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? >> >> I haven't seen any complains, but I'll double check. > > Might be worth it to cc Greg on this, since the device driver model > has a lot of corner cases to make sure it doesn't get stuck here with > hiararchical drivers/device getting removed while something else is > probing them at the same time. Oh, OK. I'll keep that in mind when sending out v2 of these patches. Best regards Thomas > -Daniel > >> Best regards >> Thomas >> >>> >>> 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 >> > >
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@suse.de> > > 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 >> >
On Tue, Sep 29, 2020 at 10:59:10AM +0200, Thomas Zimmermann wrote: > 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@suse.de> > > > > 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. Simple because the only reason you might want to use this is for display. In no other case do we load a firmware driver to make hw semi-useable. Putting it where people might look for it and all that. -Daniel > > 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 >
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
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@suse.de> --- 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