On Sat, Nov 04, 2017 at 02:03:56PM +0100, Noralf Trønnes wrote:
> Add functions drm_fb_cma_fbdev_init(), drm_fb_cma_fbdev_fini() and
> drm_fb_cma_fbdev_init_with_funcs(). These functions relies on the fact
> that the drm_fb_helper struct is stored in dev->drm_fb_helper_private
> so drivers don't need to store it.
>
> Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
I guess you've proven me wrong, since at the end of this patch series we
do have some neat functions that do the same thing an earlier patch series
did, expect there's a _cma_ in the name. But the function itself is 100%
generic.
Sry for all the meandering, I guess this once again shows that any problem
in software can indeed be solved by adding another abstraction layer on
top. You'll probably be slightly mad, but I'd vote for the patch to drop
the _cma_ infix again (once everything has landed, no need to respin all
the patches again).
> ---
> drivers/gpu/drm/drm_fb_cma_helper.c | 116 +++++++++++++++++++++++++++++++++++-
> include/drm/drm_fb_cma_helper.h | 7 +++
> 2 files changed, 121 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 0e3c14174d08..267c04216281 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -23,6 +23,7 @@
> #include <drm/drm_gem_cma_helper.h>
> #include <drm/drm_gem_framebuffer_helper.h>
> #include <drm/drm_fb_cma_helper.h>
> +#include <drm/drm_print.h>
> #include <linux/module.h>
>
> #define DEFAULT_FBDEFIO_DELAY_MS 50
> @@ -42,7 +43,7 @@ struct drm_fbdev_cma {
> * callback function to create a cma backed framebuffer.
> *
> * An fbdev framebuffer backed by cma is also available by calling
> - * drm_fbdev_cma_init(). drm_fbdev_cma_fini() tears it down.
> + * drm_fb_cma_fbdev_init(). drm_fb_cma_fbdev_fini() tears it down.
> * If the &drm_framebuffer_funcs.dirty callback is set, fb_deferred_io will be
> * set up automatically. &drm_framebuffer_funcs.dirty is called by
> * drm_fb_helper_deferred_io() in process context (&struct delayed_work).
> @@ -68,7 +69,7 @@ struct drm_fbdev_cma {
> *
> * Initialize::
> *
> - * fbdev = drm_fbdev_cma_init_with_funcs(dev, 16,
> + * fbdev = drm_fb_cma_fbdev_init_with_funcs(dev, 16,
> * dev->mode_config.num_crtc,
> * dev->mode_config.num_connector,
> * &driver_fb_funcs);
> @@ -314,6 +315,117 @@ static const struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = {
> .fb_probe = drm_fbdev_cma_create,
> };
>
> +/**
> + * drm_fb_cma_fbdev_init_with_funcs() - Allocate and initialize fbdev emulation
> + * @dev: DRM device
> + * @preferred_bpp: Preferred bits per pixel for the device.
> + * @dev->mode_config.preferred_depth is used if this is zero.
> + * @max_conn_count: Maximum number of connectors.
> + * @dev->mode_config.num_connector is used if this is zero.
> + * @funcs: Framebuffer functions, in particular a custom dirty() callback.
With the previous patch this is allowed to be NULL, right? Please
document.
And since that also explains the patch 1, on both patches:
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
But please get an ack from Laurent too.
-Daniel
> + *
> + * Returns:
> + * Zero on success or negative error code on failure.
> + */
> +int drm_fb_cma_fbdev_init_with_funcs(struct drm_device *dev,
> + unsigned int preferred_bpp, unsigned int max_conn_count,
> + const struct drm_framebuffer_funcs *funcs)
> +{
> + struct drm_fbdev_cma *fbdev_cma;
> + struct drm_fb_helper *fb_helper;
> + int ret;
> +
> + if (!preferred_bpp)
> + preferred_bpp = dev->mode_config.preferred_depth;
> + if (!preferred_bpp)
> + preferred_bpp = 32;
> +
> + if (!max_conn_count)
> + max_conn_count = dev->mode_config.num_connector;
> +
> + fbdev_cma = kzalloc(sizeof(*fbdev_cma), GFP_KERNEL);
> + if (!fbdev_cma)
> + return -ENOMEM;
> +
> + fbdev_cma->fb_funcs = funcs;
> + fb_helper = &fbdev_cma->fb_helper;
> +
> + drm_fb_helper_prepare(dev, fb_helper, &drm_fb_cma_helper_funcs);
> +
> + ret = drm_fb_helper_init(dev, fb_helper, max_conn_count);
> + if (ret < 0) {
> + DRM_DEV_ERROR(dev->dev, "Failed to initialize fbdev helper.\n");
> + goto err_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, preferred_bpp);
> + if (ret < 0) {
> + DRM_DEV_ERROR(dev->dev, "Failed to set fbdev configuration.\n");
> + goto err_drm_fb_helper_fini;
> + }
> +
> + return 0;
> +
> +err_drm_fb_helper_fini:
> + drm_fb_helper_fini(fb_helper);
> +err_free:
> + kfree(fbdev_cma);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(drm_fb_cma_fbdev_init_with_funcs);
> +
> +/**
> + * drm_fb_cma_fbdev_init() - Allocate and initialize fbdev emulation
> + * @dev: DRM device
> + * @preferred_bpp: Preferred bits per pixel for the device.
> + * @dev->mode_config.preferred_depth is used if this is zero.
> + * @max_conn_count: Maximum number of connectors.
> + * @dev->mode_config.num_connector is used if this is zero.
> + *
> + * Returns:
> + * Zero on success or negative error code on failure.
> + */
> +int drm_fb_cma_fbdev_init(struct drm_device *dev, unsigned int preferred_bpp,
> + unsigned int max_conn_count)
> +{
> + return drm_fb_cma_fbdev_init_with_funcs(dev, preferred_bpp,
> + max_conn_count, NULL);
> +}
> +EXPORT_SYMBOL_GPL(drm_fb_cma_fbdev_init);
> +
> +/**
> + * drm_fb_cma_fbdev_fini() - Teardown fbdev emulation
> + * @dev: DRM device
> + */
> +void drm_fb_cma_fbdev_fini(struct drm_device *dev)
> +{
> + struct drm_fb_helper *fb_helper = dev->fb_helper;
> +
> + if (!fb_helper)
> + return;
> +
> + /* Unregister if it hasn't been done already */
> + if (fb_helper->fbdev && fb_helper->fbdev->dev)
> + drm_fb_helper_unregister_fbi(fb_helper);
> +
> + if (fb_helper->fbdev)
> + drm_fbdev_cma_defio_fini(fb_helper->fbdev);
> +
> + if (fb_helper->fb)
> + drm_framebuffer_remove(fb_helper->fb);
> +
> + drm_fb_helper_fini(fb_helper);
> + kfree(to_fbdev_cma(fb_helper));
> +}
> +EXPORT_SYMBOL_GPL(drm_fb_cma_fbdev_fini);
> +
> /**
> * drm_fbdev_cma_init_with_funcs() - Allocate and initializes a drm_fbdev_cma struct
> * @dev: DRM device
> diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h
> index 023f052a5873..22be22e641a4 100644
> --- a/include/drm/drm_fb_cma_helper.h
> +++ b/include/drm/drm_fb_cma_helper.h
> @@ -15,6 +15,13 @@ struct drm_mode_fb_cmd2;
> struct drm_plane;
> struct drm_plane_state;
>
> +int drm_fb_cma_fbdev_init_with_funcs(struct drm_device *dev,
> + unsigned int preferred_bpp, unsigned int max_conn_count,
> + const struct drm_framebuffer_funcs *funcs);
> +int drm_fb_cma_fbdev_init(struct drm_device *dev, unsigned int preferred_bpp,
> + unsigned int max_conn_count);
> +void drm_fb_cma_fbdev_fini(struct drm_device *dev);
> +
> struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
> unsigned int preferred_bpp, unsigned int max_conn_count,
> const struct drm_framebuffer_funcs *funcs);
> --
> 2.14.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
