From: Thomas Zimmermann <tzimmermann@suse.de>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: geert+renesas@glider.be, airlied@linux.ie,
emil.l.velikov@gmail.com, dri-devel@lists.freedesktop.org,
lgirdwood@gmail.com, hdegoede@redhat.com, broonie@kernel.org,
kraxel@redhat.com, sam@ravnborg.org
Subject: Re: [PATCH 8/9] drm: Add infrastructure for platform devices
Date: Tue, 29 Sep 2020 10:59:10 +0200 [thread overview]
Message-ID: <e3eb13e0-a1a6-69ef-f615-ebec21f326af@suse.de> (raw)
In-Reply-To: <20200629092741.GR3278063@phenom.ffwll.local>
[-- Attachment #1.1.1: Type: text/plain, Size: 9928 bytes --]
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
>>
>
--
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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 516 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-09-29 8:59 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-25 12:00 [RFC][PATCH 0/9] drm: Support simple-framebuffer devices and firmware fbs Thomas Zimmermann
2020-06-25 12:00 ` [PATCH 1/9] drm/format-helper: Pass destination pitch to drm_fb_memcpy_dstclip() Thomas Zimmermann
2020-06-29 8:40 ` Daniel Vetter
2020-09-25 14:55 ` Thomas Zimmermann
2020-09-26 16:42 ` Daniel Vetter
2020-09-28 7:22 ` Thomas Zimmermann
2020-09-28 8:53 ` Daniel Vetter
2020-09-28 9:13 ` Thomas Zimmermann
2020-09-29 9:19 ` Daniel Vetter
2020-09-29 9:39 ` Thomas Zimmermann
2020-09-29 11:32 ` Daniel Vetter
2020-09-28 10:24 ` Gerd Hoffmann
2020-09-28 13:42 ` Pekka Paalanen
2020-06-25 12:00 ` [PATCH 2/9] drm/format-helper: Add blitter functions Thomas Zimmermann
2020-06-29 8:46 ` Daniel Vetter
2020-06-25 12:00 ` [PATCH 3/9] drm: Add simplekms driver Thomas Zimmermann
2020-06-29 9:06 ` Daniel Vetter
2020-09-25 15:01 ` Thomas Zimmermann
2020-09-25 15:14 ` Maxime Ripard
2020-09-28 7:25 ` Thomas Zimmermann
2021-02-10 16:14 ` Thomas Zimmermann
2020-06-25 12:00 ` [PATCH 4/9] drm/simplekms: Add fbdev emulation Thomas Zimmermann
2020-06-29 9:11 ` Daniel Vetter
2020-06-25 12:00 ` [PATCH 5/9] drm/simplekms: Initialize framebuffer data from device-tree node Thomas Zimmermann
2020-06-30 2:36 ` Rob Herring
2020-06-25 12:00 ` [PATCH 6/9] drm/simplekms: Acquire clocks from DT device node Thomas Zimmermann
2020-06-25 13:34 ` Geert Uytterhoeven
2020-06-29 9:07 ` Daniel Vetter
2020-06-25 12:00 ` [PATCH 7/9] drm/simplekms: Acquire regulators " Thomas Zimmermann
2020-06-25 13:36 ` Geert Uytterhoeven
2020-06-25 12:00 ` [PATCH 8/9] drm: Add infrastructure for platform devices Thomas Zimmermann
2020-06-29 9:27 ` Daniel Vetter
2020-09-28 8:40 ` Thomas Zimmermann
2020-09-28 8:50 ` Daniel Vetter
2020-09-28 9:14 ` Thomas Zimmermann
2020-09-29 8:59 ` Thomas Zimmermann [this message]
2020-09-29 9:20 ` Daniel Vetter
2020-06-30 9:11 ` Daniel Vetter
2020-06-25 12:00 ` [PATCH 9/9] drm/simplekms: Acquire memory aperture for framebuffer Thomas Zimmermann
2020-06-29 9:22 ` Daniel Vetter
2020-06-29 16:04 ` Greg KH
2020-06-29 16:23 ` Mark Brown
2020-06-29 16:57 ` Greg KH
2020-06-30 2:13 ` Rob Herring
2020-06-30 8:50 ` Greg KH
2020-06-29 9:38 ` [RFC][PATCH 0/9] drm: Support simple-framebuffer devices and firmware fbs Hans de Goede
2020-06-30 9:06 ` Daniel Vetter
2020-06-30 9:13 ` Hans de Goede
2020-07-01 14:10 ` Thomas Zimmermann
2020-07-03 10:55 ` Hans de Goede
2020-07-03 11:42 ` Thomas Zimmermann
2020-07-03 12:58 ` Daniel Vetter
2020-07-03 14:11 ` Hans de Goede
2020-07-01 13:48 ` Thomas Zimmermann
2020-07-03 10:44 ` Hans de Goede
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e3eb13e0-a1a6-69ef-f615-ebec21f326af@suse.de \
--to=tzimmermann@suse.de \
--cc=airlied@linux.ie \
--cc=broonie@kernel.org \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=emil.l.velikov@gmail.com \
--cc=geert+renesas@glider.be \
--cc=hdegoede@redhat.com \
--cc=kraxel@redhat.com \
--cc=lgirdwood@gmail.com \
--cc=sam@ravnborg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).