Re: [PATCH v2 03/10] drm/aperture: Move fbdev conflict helpers into drm_aperture.h
Hi
Am 08.04.21 um 11:50 schrieb Daniel Vetter:
On Thu, Mar 18, 2021 at 11:29:14AM +0100, Thomas Zimmermann wrote:
Fbdev's helpers for handling conflicting framebuffers are related to
framebuffer apertures, not console emulation. Therefore move them into a
drm_aperture.h, which will contain the interfaces for the new aperture
helpers.
Signed-off-by: Thomas Zimmermann
Tested-by: nerdopolis
---
Documentation/gpu/drm-internals.rst | 6 +++
include/drm/drm_aperture.h | 60 +
include/drm/drm_fb_helper.h | 56 ++-
3 files changed, 69 insertions(+), 53 deletions(-)
create mode 100644 include/drm/drm_aperture.h
diff --git a/Documentation/gpu/drm-internals.rst
b/Documentation/gpu/drm-internals.rst
index 12272b168580..4c7642d2ca34 100644
--- a/Documentation/gpu/drm-internals.rst
+++ b/Documentation/gpu/drm-internals.rst
@@ -75,6 +75,12 @@ update it, its value is mostly useless. The DRM core prints
it to the
kernel log at initialization time and passes it to userspace through the
DRM_IOCTL_VERSION ioctl.
+Managing Ownership of the Framebuffer Aperture
+--
+
+.. kernel-doc:: include/drm/drm_aperture.h
+ :internal:
+
Device Instance and Driver Handling
---
diff --git a/include/drm/drm_aperture.h b/include/drm/drm_aperture.h
new file mode 100644
index ..13766efe9517
--- /dev/null
+++ b/include/drm/drm_aperture.h
@@ -0,0 +1,60 @@
+/* SPDX-License-Identifier: MIT */
+
+#ifndef _DRM_APERTURE_H_
+#define _DRM_APERTURE_H_
+
+#include
+#include
+
+/**
+ * drm_fb_helper_remove_conflicting_framebuffers - remove firmware-configured
framebuffers
Annoying bikeshed, but I'd give them drm_aperture_ prefixes, for ocd
consistency. Also make them real functions, they're quite big and will
grow more in the next patch.
I'm also not super happy about the naming here but oh well.
The original name for this was platform helpers, which was worse. So
it's not like we're not improving. :)
I'll take this patch + some docs from patch 4 + your feedback and turn
it into a separate patchset. It should be useful even without simpledrm.
Best regards
Thomas
Either way: Acked-by: Daniel Vetter
+ * @a: memory range, users of which are to be removed
+ * @name: requesting driver name
+ * @primary: also kick vga16fb if present
+ *
+ * This function removes framebuffer devices (initialized by
firmware/bootloader)
+ * which use memory range described by @a. If @a is NULL all such devices are
+ * removed.
+ */
+static inline int
+drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a,
+ const char *name, bool primary)
+{
+#if IS_REACHABLE(CONFIG_FB)
+ return remove_conflicting_framebuffers(a, name, primary);
+#else
+ return 0;
+#endif
+}
+
+/**
+ * drm_fb_helper_remove_conflicting_pci_framebuffers - remove
firmware-configured
+ * framebuffers for PCI
devices
+ * @pdev: PCI device
+ * @name: requesting driver name
+ *
+ * This function removes framebuffer devices (eg. initialized by firmware)
+ * using memory range configured for any of @pdev's memory bars.
+ *
+ * The function assumes that PCI device with shadowed ROM drives a primary
+ * display and so kicks out vga16fb.
+ */
+static inline int
+drm_fb_helper_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
+ const char *name)
+{
+ int ret = 0;
+
+ /*
+* WARNING: Apparently we must kick fbdev drivers before vgacon,
+* otherwise the vga fbdev driver falls over.
+*/
+#if IS_REACHABLE(CONFIG_FB)
+ ret = remove_conflicting_pci_framebuffers(pdev, name);
+#endif
+ if (ret == 0)
+ ret = vga_remove_vgacon(pdev);
+ return ret;
+}
+
+#endif
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 3b273f9ca39a..d06a3942fddb 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -30,13 +30,13 @@
#ifndef DRM_FB_HELPER_H
#define DRM_FB_HELPER_H
-struct drm_fb_helper;
-
+#include
#include
#include
#include
#include
-#include
+
+struct drm_fb_helper;
enum mode_set_atomic {
LEAVE_ATOMIC_MODE_SET,
@@ -451,54 +451,4 @@ drm_fbdev_generic_setup(struct drm_device *dev, unsigned
int preferred_bpp)
#endif
-/**
- * drm_fb_helper_remove_conflicting_framebuffers - remove firmware-configured
framebuffers
- * @a: memory range, users of which are to be removed
- * @name: requesting driver name
- * @primary: also kick vga16fb if present
- *
- * This function removes framebuffer devices (initialized by
firmware/bootloader)
- * which use memory range described by @a. If @a is NULL all such devices are
- * removed.
- */
-static inline int
-drm_fb_helper_remove_conflicting_framebuffers(struct ape
Re: [PATCH v2 03/10] drm/aperture: Move fbdev conflict helpers into drm_aperture.h
On Thu, Mar 18, 2021 at 11:29:14AM +0100, Thomas Zimmermann wrote:
> Fbdev's helpers for handling conflicting framebuffers are related to
> framebuffer apertures, not console emulation. Therefore move them into a
> drm_aperture.h, which will contain the interfaces for the new aperture
> helpers.
>
> Signed-off-by: Thomas Zimmermann
> Tested-by: nerdopolis
> ---
> Documentation/gpu/drm-internals.rst | 6 +++
> include/drm/drm_aperture.h | 60 +
> include/drm/drm_fb_helper.h | 56 ++-
> 3 files changed, 69 insertions(+), 53 deletions(-)
> create mode 100644 include/drm/drm_aperture.h
>
> diff --git a/Documentation/gpu/drm-internals.rst
> b/Documentation/gpu/drm-internals.rst
> index 12272b168580..4c7642d2ca34 100644
> --- a/Documentation/gpu/drm-internals.rst
> +++ b/Documentation/gpu/drm-internals.rst
> @@ -75,6 +75,12 @@ update it, its value is mostly useless. The DRM core
> prints it to the
> kernel log at initialization time and passes it to userspace through the
> DRM_IOCTL_VERSION ioctl.
>
> +Managing Ownership of the Framebuffer Aperture
> +--
> +
> +.. kernel-doc:: include/drm/drm_aperture.h
> + :internal:
> +
> Device Instance and Driver Handling
> ---
>
> diff --git a/include/drm/drm_aperture.h b/include/drm/drm_aperture.h
> new file mode 100644
> index ..13766efe9517
> --- /dev/null
> +++ b/include/drm/drm_aperture.h
> @@ -0,0 +1,60 @@
> +/* SPDX-License-Identifier: MIT */
> +
> +#ifndef _DRM_APERTURE_H_
> +#define _DRM_APERTURE_H_
> +
> +#include
> +#include
> +
> +/**
> + * drm_fb_helper_remove_conflicting_framebuffers - remove
> firmware-configured framebuffers
Annoying bikeshed, but I'd give them drm_aperture_ prefixes, for ocd
consistency. Also make them real functions, they're quite big and will
grow more in the next patch.
I'm also not super happy about the naming here but oh well.
Either way: Acked-by: Daniel Vetter
> + * @a: memory range, users of which are to be removed
> + * @name: requesting driver name
> + * @primary: also kick vga16fb if present
> + *
> + * This function removes framebuffer devices (initialized by
> firmware/bootloader)
> + * which use memory range described by @a. If @a is NULL all such devices are
> + * removed.
> + */
> +static inline int
> +drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a,
> + const char *name, bool primary)
> +{
> +#if IS_REACHABLE(CONFIG_FB)
> + return remove_conflicting_framebuffers(a, name, primary);
> +#else
> + return 0;
> +#endif
> +}
> +
> +/**
> + * drm_fb_helper_remove_conflicting_pci_framebuffers - remove
> firmware-configured
> + * framebuffers for PCI
> devices
> + * @pdev: PCI device
> + * @name: requesting driver name
> + *
> + * This function removes framebuffer devices (eg. initialized by firmware)
> + * using memory range configured for any of @pdev's memory bars.
> + *
> + * The function assumes that PCI device with shadowed ROM drives a primary
> + * display and so kicks out vga16fb.
> + */
> +static inline int
> +drm_fb_helper_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
> + const char *name)
> +{
> + int ret = 0;
> +
> + /*
> + * WARNING: Apparently we must kick fbdev drivers before vgacon,
> + * otherwise the vga fbdev driver falls over.
> + */
> +#if IS_REACHABLE(CONFIG_FB)
> + ret = remove_conflicting_pci_framebuffers(pdev, name);
> +#endif
> + if (ret == 0)
> + ret = vga_remove_vgacon(pdev);
> + return ret;
> +}
> +
> +#endif
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 3b273f9ca39a..d06a3942fddb 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -30,13 +30,13 @@
> #ifndef DRM_FB_HELPER_H
> #define DRM_FB_HELPER_H
>
> -struct drm_fb_helper;
> -
> +#include
> #include
> #include
> #include
> #include
> -#include
> +
> +struct drm_fb_helper;
>
> enum mode_set_atomic {
> LEAVE_ATOMIC_MODE_SET,
> @@ -451,54 +451,4 @@ drm_fbdev_generic_setup(struct drm_device *dev, unsigned
> int preferred_bpp)
>
> #endif
>
> -/**
> - * drm_fb_helper_remove_conflicting_framebuffers - remove
> firmware-configured framebuffers
> - * @a: memory range, users of which are to be removed
> - * @name: requesting driver name
> - * @primary: also kick vga16fb if present
> - *
> - * This function removes framebuffer devices (initialized by
> firmware/bootloader)
> - * which use memory range described by @a. If @a is NULL all such devices are
> - * removed.
> - */
> -static inline int
> -drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a,
> - const char *name, bool primary)
> -{
> -
[PATCH v2 03/10] drm/aperture: Move fbdev conflict helpers into drm_aperture.h
Fbdev's helpers for handling conflicting framebuffers are related to
framebuffer apertures, not console emulation. Therefore move them into a
drm_aperture.h, which will contain the interfaces for the new aperture
helpers.
Signed-off-by: Thomas Zimmermann
Tested-by: nerdopolis
---
Documentation/gpu/drm-internals.rst | 6 +++
include/drm/drm_aperture.h | 60 +
include/drm/drm_fb_helper.h | 56 ++-
3 files changed, 69 insertions(+), 53 deletions(-)
create mode 100644 include/drm/drm_aperture.h
diff --git a/Documentation/gpu/drm-internals.rst
b/Documentation/gpu/drm-internals.rst
index 12272b168580..4c7642d2ca34 100644
--- a/Documentation/gpu/drm-internals.rst
+++ b/Documentation/gpu/drm-internals.rst
@@ -75,6 +75,12 @@ update it, its value is mostly useless. The DRM core prints
it to the
kernel log at initialization time and passes it to userspace through the
DRM_IOCTL_VERSION ioctl.
+Managing Ownership of the Framebuffer Aperture
+--
+
+.. kernel-doc:: include/drm/drm_aperture.h
+ :internal:
+
Device Instance and Driver Handling
---
diff --git a/include/drm/drm_aperture.h b/include/drm/drm_aperture.h
new file mode 100644
index ..13766efe9517
--- /dev/null
+++ b/include/drm/drm_aperture.h
@@ -0,0 +1,60 @@
+/* SPDX-License-Identifier: MIT */
+
+#ifndef _DRM_APERTURE_H_
+#define _DRM_APERTURE_H_
+
+#include
+#include
+
+/**
+ * drm_fb_helper_remove_conflicting_framebuffers - remove firmware-configured
framebuffers
+ * @a: memory range, users of which are to be removed
+ * @name: requesting driver name
+ * @primary: also kick vga16fb if present
+ *
+ * This function removes framebuffer devices (initialized by
firmware/bootloader)
+ * which use memory range described by @a. If @a is NULL all such devices are
+ * removed.
+ */
+static inline int
+drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a,
+ const char *name, bool primary)
+{
+#if IS_REACHABLE(CONFIG_FB)
+ return remove_conflicting_framebuffers(a, name, primary);
+#else
+ return 0;
+#endif
+}
+
+/**
+ * drm_fb_helper_remove_conflicting_pci_framebuffers - remove
firmware-configured
+ * framebuffers for PCI
devices
+ * @pdev: PCI device
+ * @name: requesting driver name
+ *
+ * This function removes framebuffer devices (eg. initialized by firmware)
+ * using memory range configured for any of @pdev's memory bars.
+ *
+ * The function assumes that PCI device with shadowed ROM drives a primary
+ * display and so kicks out vga16fb.
+ */
+static inline int
+drm_fb_helper_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
+ const char *name)
+{
+ int ret = 0;
+
+ /*
+* WARNING: Apparently we must kick fbdev drivers before vgacon,
+* otherwise the vga fbdev driver falls over.
+*/
+#if IS_REACHABLE(CONFIG_FB)
+ ret = remove_conflicting_pci_framebuffers(pdev, name);
+#endif
+ if (ret == 0)
+ ret = vga_remove_vgacon(pdev);
+ return ret;
+}
+
+#endif
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 3b273f9ca39a..d06a3942fddb 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -30,13 +30,13 @@
#ifndef DRM_FB_HELPER_H
#define DRM_FB_HELPER_H
-struct drm_fb_helper;
-
+#include
#include
#include
#include
#include
-#include
+
+struct drm_fb_helper;
enum mode_set_atomic {
LEAVE_ATOMIC_MODE_SET,
@@ -451,54 +451,4 @@ drm_fbdev_generic_setup(struct drm_device *dev, unsigned
int preferred_bpp)
#endif
-/**
- * drm_fb_helper_remove_conflicting_framebuffers - remove firmware-configured
framebuffers
- * @a: memory range, users of which are to be removed
- * @name: requesting driver name
- * @primary: also kick vga16fb if present
- *
- * This function removes framebuffer devices (initialized by
firmware/bootloader)
- * which use memory range described by @a. If @a is NULL all such devices are
- * removed.
- */
-static inline int
-drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a,
- const char *name, bool primary)
-{
-#if IS_REACHABLE(CONFIG_FB)
- return remove_conflicting_framebuffers(a, name, primary);
-#else
- return 0;
-#endif
-}
-
-/**
- * drm_fb_helper_remove_conflicting_pci_framebuffers - remove
firmware-configured framebuffers for PCI devices
- * @pdev: PCI device
- * @name: requesting driver name
- *
- * This function removes framebuffer devices (eg. initialized by firmware)
- * using memory range configured for any of @pdev's memory bars.
- *
- * The function assumes that PCI device with shadowed ROM drives a primary
- * display and so kicks out vga16fb.
- */
-static inline int
-drm_
