Hi
Am 13.03.24 um 10:03 schrieb Geert Uytterhoeven:
Hi Thomas,
On Wed, Mar 13, 2024 at 9:19 AM Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
Am 12.03.24 um 17:14 schrieb Geert Uytterhoeven:
On Tue, Mar 12, 2024 at 4:48 PM Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
Add an fbdev emulation for SHMEM-based memory managers. The code is
similar to fbdev-generic, but does not require an addition shadow
buffer for mmap(). Fbdev-shmem operates directly on the buffer object's
SHMEM pages. Fbdev's deferred-I/O mechanism updates the hardware state
on write operations.
Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
Thanks for your patch!
--- /dev/null
+++ b/drivers/gpu/drm/drm_fbdev_shmem.c
+static int drm_fbdev_shmem_helper_fb_probe(struct drm_fb_helper *fb_helper,
+ struct drm_fb_helper_surface_size *sizes)
+{
+ struct drm_client_dev *client = &fb_helper->client;
+ struct drm_device *dev = fb_helper->dev;
+ struct drm_client_buffer *buffer;
+ struct drm_gem_shmem_object *shmem;
+ struct drm_framebuffer *fb;
+ struct fb_info *info;
+ u32 format;
+ struct iosys_map map;
+ int ret;
+
+ drm_dbg_kms(dev, "surface width(%d), height(%d) and bpp(%d)\n",
+ sizes->surface_width, sizes->surface_height,
+ sizes->surface_bpp);
+
+ format = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth);
Oops, one more caller of the imprecise
let's-guess-the-format-from-bpp-and-depth machinery to get rid of...
Right, that has been discussed in another thread. I'll change this call
to the drm_driver_() function.
You mean drm_driver_legacy_fb_format()? That has the same issues.
We have the video= parameter with its bpp argument. So we won't ever
fully remove that function.
+ buffer = drm_client_framebuffer_create(client, sizes->surface_width,
+ sizes->surface_height, format);
[...]
+}
+/**
+ * drm_fbdev_shmem_setup() - Setup fbdev emulation for GEM SHMEM helpers
+ * @dev: DRM device
+ * @preferred_bpp: Preferred bits per pixel for the device.
+ * 32 is used if this is zero.
+ *
+ * This function sets up fbdev emulation for GEM DMA drivers that support
+ * dumb buffers with a virtual address and that can be mmap'ed.
+ * drm_fbdev_shmem_setup() shall be called after the DRM driver registered
+ * the new DRM device with drm_dev_register().
+ *
+ * Restore, hotplug events and teardown are all taken care of. Drivers that do
+ * suspend/resume need to call drm_fb_helper_set_suspend_unlocked() themselves.
+ * Simple drivers might use drm_mode_config_helper_suspend().
+ *
+ * This function is safe to call even when there are no connectors present.
+ * Setup will be retried on the next hotplug event.
+ *
+ * The fbdev is destroyed by drm_dev_unregister().
+ */
+void drm_fbdev_shmem_setup(struct drm_device *dev, unsigned int preferred_bpp)
As this is a completely new function, can we please get a
preferred_format parameter instead?
An understandable question. But as it is, the patchset has a trivial
change in each driver. And the preferred_bpp parameter has the same
meaning as the bpp value in the video= parameter. So it's ok-ish for now.
OK.
Using a format parameter here is really a much larger update and touches
the internals of the fbdev emulation. I'm not even sure that we should
have a parameter at all. Since in-kernel clients should behave like
userspace clients, we could try to figure out the format from the
driver's primary planes. That's a patchset of its own.
How do you figure out "the" format from the driver's primary plane?
Isn't that a list of formats (always including XR24) , so the driver
still needs to specify a preferred format?
The list of formats for each plane is roughly sorted by preference. We
can go through it and pick the first format that is supported by the
fbdev code. That's likely how userspace would do it.
A while ago, I had a look into replacing preferred_{depth,bpp} by
preferred_format, but I was held back by the inconsistencies in some
drivers (e.g. depth 24 vs. 32). Perhaps an incremental approach
(use preferred_format if available, else fall back to legacy
preferred_{depth,bpp} handling) would be more suitable?
I have initial patches to move format selection from the fb_probe
helpers into the shared helpers. That allows to remove the surface_depth
and surface_bpp fields. That is at least a step into the right direction.
FTR, my main use-case is letting fbdev emulation distinguish between
DRM_FORMAT_Rx and DRM_FORMAT_Cx, which share the values of depth
and bpp.
How are they used for the framebuffer console? Shouldn't it always be
_Cx? _Rx is just monochrome AFAIU.
Best regards
Thomas
Gr{oetje,eeting}s,
Geert
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
