[PATCH 02/22] drm/cma-helper: Add drm_fb_cma_fbdev_init/fini()
Noralf Trønnes
noralf at tronnes.org
Mon Nov 6 12:40:41 UTC 2017
Den 06.11.2017 10.04, skrev Daniel Vetter:
> 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 at ideasonboard.com>
>> Signed-off-by: Noralf Trønnes <noralf at tronnes.org>
> 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).
>
The purpose here is not to add a layer, but to change an existing one.
I want to get rid of struct drm_fbdev_cma and I have 2 options:
- One patch that touches all drivers
- Add new functions, switch over drivers one patch at a time
In effect this is what I do in this patchset:
include/drm/drm_fb_cma_helper.h:
+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);
-struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev,
- unsigned int preferred_bpp, unsigned int max_conn_count);
-void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma);
-
How about this commit message:
drm/cma-helper: Add drm_fb_cma_fbdev_init/fini()
There is no need for drivers to carry a pointer to the the fbdev
emulation structure now that drm_device has one. Currently drivers has
a pointer to struct drm_fbdev_cma in their driver structure. This
pointer is returned by drm_fbdev_cma_init().
In order to get rid of struct drm_fbdev_cma, 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 drm_device->fb_helper so
drivers don't need to store it. When drivers have been moved over to
these functions, the old init/fini functions can be removed.
Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Signed-off-by: Noralf Trønnes <noralf at tronnes.org>
Noralf.
>> ---
>> 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 at ffwll.ch>
>
> 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
More information about the dri-devel
mailing list