Den 16.09.2017 14.37, skrev Noralf Trønnes:
Den 15.09.2017 18.37, skrev Noralf Trønnes:
Den 15.09.2017 00.29, skrev Laurent Pinchart:
Hi Noralf,
On Wednesday, 13 September 2017 18:19:22 EEST Noralf Trønnes wrote:
Den 13.09.2017 07.09, skrev Laurent Pinchart:
On Monday, 11 September 2017 17:31:54 EEST Noralf Trønnes wrote:
Hi,
I want to start out by saying that this patchset is low priority
for me
and if no one has interest or time to review this, that is just
fine. I
was in the flow and just typed it out.
This patchset adds a way for fbdev emulation code to create a
framebuffer that is backed by a dumb buffer. drm_fb_helper gets a
drm_file to hang the objects on, drm_framebuffer_create_dumb()
creates
the framebuffer and drm_fb_helper_fini() destroys it.
I have verified that all cma drivers supports dumb buffers, so
converting the library should be fine for all.
Stupid question, what does this give us ? The series makes the
call stack
more complex (up to a point where I'm getting trouble just
following it),
what's the advantage that offsets that ?
The short answer is that it avoids the need for special fbdev
_with_funcs() functions in libraries like cma for framebuffers with
the
dirty callback set.
I'm suprised that you think this more complex, I find it more coherent
when fbdev userspace is doing things more like DRM userspace, like
hanging the objects on a drm_file and cleaning up the objects when
done.
Maybe moving to a new code base gave me the wrong feeling that the
result is
more complex. The current implementation is certainly not simple.
The longer and more diffuse answer is that it's annoying me that many
drivers carry the burden of fbdev emulation just to get a console!
I totally agree with that. Ideally I'd like to remove 100% of
fbdev-related
code from drivers. This includes
- initialization and cleanup of fbdev helpers
- fbdev restore in last_close()
- forwarding of hotplug events to fbdev compatibility layer
In practice we'll likely need to keep one initialization function (or a
drm_driver flag) to let drivers opt-in to fbdev compatibility, but
the rest
should go away. Or maybe we could even enable fbdev compatibility in
all
drivers unconditionally.
Interesting idea, maybe something like this:
I just remembered that Daniel said something about the helpers shouldn't
bleed into the core, which my suggestion does :-/
struct drm_device {
+ struct drm_fb_helper *fbdev;
};
However, other helpers are allowed pointers in core structures, so if we
can do that we can at least add helpers that can be used directly as
callbacks:
/**
* drm_fb_helper_lastclose - DRM driver lastclose helper for fbdev
* @dev: DRM device
*
* This function can be used as the &drm_driver->lastclose callback for
drivers
* that only need to call drm_fb_helper_restore_fbdev_mode_unlocked().
*
* Note: &drm_device->fbdev needs to be set.
*/
void drm_fb_helper_lastclose(struct drm_device *dev)
{
drm_fb_helper_restore_fbdev_mode_unlocked(dev->fbdev);
}
EXPORT_SYMBOL(drm_fb_helper_lastclose);
/**
* drm_fb_helper_output_poll_changed - DRM mode config
\.output_poll_changed
* helper
* @dev: DRM device
*
* This function can be used as the
* &drm_mode_config_funcs.output_poll_changed callback for drivers that
only
* need to call drm_fb_helper_hotplug_event().
*
* Note: &drm_device->fbdev needs to be set.
*/
void drm_fb_helper_output_poll_changed(struct drm_device *dev)
{
drm_fb_helper_hotplug_event(dev->fbdev);
}
EXPORT_SYMBOL(drm_fb_helper_output_poll_changed);
void drm_lastclose(struct drm_device * dev)
{
DRM_DEBUG("\n");
if (dev->driver->lastclose)
dev->driver->lastclose(dev);
+ else if (dev->fbdev)
+ drm_fb_helper_restore_fbdev_mode_unlocked(dev->fbdev);
DRM_DEBUG("driver lastclose completed\n");
if (drm_core_check_feature(dev, DRIVER_LEGACY))
drm_legacy_dev_reinit(dev);
}
void drm_kms_helper_hotplug_event(struct drm_device *dev)
{
/* send a uevent + call fbdev */
drm_sysfs_hotplug_event(dev);
if (dev->mode_config.funcs->output_poll_changed)
dev->mode_config.funcs->output_poll_changed(dev);
+ else if (dev->fbdev)
+ drm_fb_helper_hotplug_event(dev->fbdev);
}
void drm_dev_unregister(struct drm_device *dev)
{
struct drm_map_list *r_list, *list_temp;
+ if (dev->fbdev)
+ drm_fb_helper_unregister_fbi(dev->fbdev);
+
drm_lastclose(dev);
...
}
static void drm_dev_release(struct kref *ref)
{
struct drm_device *dev = container_of(ref, struct drm_device, ref);
+ drm_fb_helper_simple_fini(dev);
+
if (dev->driver->release) {
dev->driver->release(dev);
} else {
drm_dev_fini(dev);
kfree(dev);
}
}
int driver_probe()
{
ret = drm_fb_helper_simple_init(dev, ...);
if (ret)
DRM_WARN("Oh well fbdev init failed, but don't let that stop
us\n");
}
I've slightly adjusted some code I have in the pipeline:
/**
* drm_fb_helper_simple_init - Simple fbdev emulation init
* @dev: DRM device
* @bpp_sel: bpp value to use for the framebuffer configuration
* @max_conn: max connector count
* @funcs: pointer to structure of functions associated with this helper
*
* Simple fbdev emulation initialization. This function allocates a
* &drm_fb_helper structure and calls:
* drm_fb_helper_prepare(), drm_fb_helper_init(),
* drm_fb_helper_single_add_all_connectors() and
* drm_fb_helper_initial_config().
*
* fbdev deferred I/O users should use drm_fb_helper_defio_init().
*
* Returns:
* Zero on success, negative error code on failure.
*/
int drm_fb_helper_simple_init(struct drm_device *dev, int bpp_sel,
int max_conn,
const struct drm_fb_helper_funcs *funcs)
{
struct drm_fb_helper *fb_helper;
int ret;
fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
if (!fb_helper)
return -ENOMEM;
drm_fb_helper_prepare(dev, fb_helper, funcs);
ret = drm_fb_helper_init(dev, fb_helper, max_conn);
if (ret < 0) {
DRM_DEV_ERROR(dev->dev, "Failed to initialize fb helper.\n");
goto err_drm_fb_helper_free;
}
ret = drm_fb_helper_single_add_all_connectors(fb_helper);
if (ret < 0) {
DRM_DEV_ERROR(dev->dev, "Failed to add connectors.\n");
goto err_drm_fb_helper_fini;
}
ret = drm_fb_helper_initial_config(fb_helper, bpp_sel);
if (ret < 0) {
DRM_DEV_ERROR(dev->dev, "Failed to set initial hw config.\n");
goto err_drm_fb_helper_fini;
}
dev->fbdev = fb_helper;
return 0;
err_drm_fb_helper_fini:
drm_fb_helper_fini(fb_helper);
err_drm_fb_helper_free:
kfree(fb_helper);
return ret;
}
EXPORT_SYMBOL_GPL(drm_fb_helper_simple_init);
/**
* drm_fb_helper_simple_fini - Simple fbdev cleanup
* @dev: DRM device
*
* Simple fbdev emulation cleanup. This function unregisters fbdev if
it is not
* done, cleans up deferred IO if necessary, removes framebuffer,
finalizes
* @fb_helper and frees the structure.
*/
void drm_fb_helper_simple_fini(struct drm_device *dev)
{
struct drm_fb_helper *fb_helper = dev->fbdev;
struct fb_ops *fbops = NULL;
struct fb_info *info;
if (!fb_helper)
return;
info = fb_helper->fbdev;
/* Make sure it hasn't been unregistered already */
if (info && info->dev)
drm_fb_helper_unregister_fbi(fb_helper);
if (info && info->fbdefio) {
fb_deferred_io_cleanup(info);
kfree(info->fbdefio);
info->fbdefio = NULL;
fbops = info->fbops;
}
drm_fb_helper_fini(fb_helper);
kfree(fbops);
if (fb_helper->fb)
drm_framebuffer_remove(fb_helper->fb);
kfree(fb_helper);
}
EXPORT_SYMBOL_GPL(drm_fb_helper_simple_fini);
<snip>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
