Re: [PATCH 11/17] drm/cma-helper: simplify setup for drivers with ->dirty callbacks

Hi Daniel,

Thank you for the patch.

On Thursday 29 Dec 2016 21:48:31 Daniel Vetter wrote:

If we store the fb funcs pointer, we can remove a bit of boilerplate. Also remove the _fbdev_ in the example code, since the fb_funcs->dirty callback has nothing to do with fbdev. It's a KMS feature, only used by the fbdev deferred_io support to implement flushing/upload.

Cc: Noralf Trønnes noralf@tronnes.org Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com

Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com

---

drivers/gpu/drm/drm_fb_cma_helper.c | 58 +++++++++++---------------------- include/drm/drm_fb_cma_helper.h | 5 +--- 2 files changed, 21 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index 76cb1aa1b089..ec081727cd5a 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -39,6 +39,7 @@ struct drm_fb_cma { struct drm_fbdev_cma { struct drm_fb_helper fb_helper; struct drm_fb_cma *fb;

• const struct drm_framebuffer_funcs *fb_funcs;

};

/** @@ -58,39 +59,29 @@ struct drm_fbdev_cma {

• Example fbdev deferred io code::

• • static int driver_fbdev_fb_dirty(struct drm_framebuffer *fb,
    

• •                                  struct drm_file *file_priv,
    

• •                                  unsigned flags, unsigned color,
    

• •                                  struct drm_clip_rect *clips,
    

• •                                  unsigned num_clips)
    

• • static int driver_fb_dirty(struct drm_framebuffer *fb,
    

• •                            struct drm_file *file_priv,
    

• •                            unsigned flags, unsigned color,
    

• •                            struct drm_clip_rect *clips,
    

• •                            unsigned num_clips)
    

  • {
    

  •     struct drm_gem_cma_object *cma = drm_fb_cma_get_gem_obj(fb, 0);
    

  •     ... push changes ...
    

  •     return 0;
    

  • }
    

• • static struct drm_framebuffer_funcs driver_fbdev_fb_funcs = {
    

• • static struct drm_framebuffer_funcs driver_fb_funcs = {
    

  •     .destroy       = drm_fb_cma_destroy,
    

  •     .create_handle = drm_fb_cma_create_handle,
    

• •     .dirty         = driver_fbdev_fb_dirty,
    

• •     .dirty         = driver_fb_dirty,
    

  • };
    

• • static int driver_fbdev_create(struct drm_fb_helper *helper,
    

• •         struct drm_fb_helper_surface_size *sizes)
    

• • {
    

• •     return drm_fbdev_cma_create_with_funcs(helper, sizes,
    

• •                                            &driver_fbdev_fb_funcs);
    

• • }
    

• • static const struct drm_fb_helper_funcs driver_fb_helper_funcs = {
    

• •     .fb_probe = driver_fbdev_create,
    

• • };
    

• • Initialize::

• • Initialize:
    

  • fbdev = drm_fbdev_cma_init_with_funcs(dev, 16,
    

  •                                       dev->mode_config.num_crtc,
    

dev->mode_config.num_connector, - * &driver_fb_helper_funcs); + * &driver_fb_funcs);

*/

@@ -408,13 +399,9 @@ static void drm_fbdev_cma_defio_fini(struct fb_info *fbi) kfree(fbi->fbops); }

-/*

• • For use in a (struct drm_fb_helper_funcs *)->fb_probe callback function

that - * needs custom struct drm_framebuffer_funcs, like dirty() for deferred_io use. - */ -int drm_fbdev_cma_create_with_funcs(struct drm_fb_helper *helper,

• struct drm_fb_helper_surface_size *sizes,
• const struct drm_framebuffer_funcs *funcs)

+static int +drm_fbdev_cma_create(struct drm_fb_helper *helper,

• struct drm_fb_helper_surface_size *sizes)

{ struct drm_fbdev_cma *fbdev_cma = to_fbdev_cma(helper); struct drm_mode_fb_cmd2 mode_cmd = { 0 }; @@ -450,7 +437,8 @@ int drm_fbdev_cma_create_with_funcs(struct drm_fb_helper *helper, goto err_gem_free_object; }

• fbdev_cma->fb = drm_fb_cma_alloc(dev, &mode_cmd, &obj, 1, funcs);

• fbdev_cma->fb = drm_fb_cma_alloc(dev, &mode_cmd, &obj, 1,

• 			 fbdev_cma->fb_funcs);
  

  if (IS_ERR(fbdev_cma->fb)) { dev_err(dev->dev, "Failed to allocate DRM framebuffer.\n"); ret = PTR_ERR(fbdev_cma->fb);

@@ -476,7 +464,7 @@ int drm_fbdev_cma_create_with_funcs(struct drm_fb_helper *helper, fbi->screen_size = size; fbi->fix.smem_len = size;

• if (funcs->dirty) {

• if (fbdev_cma->fb_funcs->dirty) { ret = drm_fbdev_cma_defio_init(fbi, obj); if (ret) goto err_cma_destroy;

@@ -492,13 +480,6 @@ int drm_fbdev_cma_create_with_funcs(struct drm_fb_helper *helper, drm_gem_object_unreference_unlocked(&obj->base); return ret; } -EXPORT_SYMBOL(drm_fbdev_cma_create_with_funcs);

-static int drm_fbdev_cma_create(struct drm_fb_helper *helper,

• struct drm_fb_helper_surface_size *sizes)

-{

• return drm_fbdev_cma_create_with_funcs(helper, sizes,

&drm_fb_cma_funcs);

-}

static const struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = { .fb_probe = drm_fbdev_cma_create, @@ -516,7 +497,7 @@ static const struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = { */ struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev, unsigned int preferred_bpp, unsigned int num_crtc,

• unsigned int max_conn_count, const struct drm_fb_helper_funcs *funcs)

• unsigned int max_conn_count, const struct drm_framebuffer_funcs

*funcs)

{ struct drm_fbdev_cma *fbdev_cma; struct drm_fb_helper *helper; @@ -527,10 +508,11 @@ struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev, dev_err(dev->dev, "Failed to allocate drm fbdev.\n"); return ERR_PTR(-ENOMEM); }

• fbdev_cma->fb_funcs = funcs;

  helper = &fbdev_cma->fb_helper;

• drm_fb_helper_prepare(dev, helper, funcs);

• drm_fb_helper_prepare(dev, helper, &drm_fb_cma_helper_funcs);

  ret = drm_fb_helper_init(dev, helper, num_crtc, max_conn_count); if (ret < 0) {

@@ -576,7 +558,7 @@ struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev, unsigned int max_conn_count) { return drm_fbdev_cma_init_with_funcs(dev, preferred_bpp, num_crtc,

• 		max_conn_count, &drm_fb_cma_helper_funcs);
  

• 		max_conn_count, &drm_fb_cma_funcs);
  

} EXPORT_SYMBOL_GPL(drm_fbdev_cma_init);

diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h index 3b00f6480b83..9f4e34ea99fd 100644 --- a/include/drm/drm_fb_cma_helper.h +++ b/include/drm/drm_fb_cma_helper.h @@ -17,7 +17,7 @@ struct drm_plane_state;

struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev, unsigned int preferred_bpp, unsigned int num_crtc,

• unsigned int max_conn_count, const struct drm_fb_helper_funcs *funcs);

• 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 num_crtc, unsigned int max_conn_count); @@ -26,9 +26,6 @@ void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma); void drm_fbdev_cma_restore_mode(struct drm_fbdev_cma *fbdev_cma); void drm_fbdev_cma_hotplug_event(struct drm_fbdev_cma *fbdev_cma); void drm_fbdev_cma_set_suspend(struct drm_fbdev_cma *fbdev_cma, int state); -int drm_fbdev_cma_create_with_funcs(struct drm_fb_helper *helper, - struct drm_fb_helper_surface_size *sizes,

• const struct drm_framebuffer_funcs *funcs);

void drm_fb_cma_destroy(struct drm_framebuffer *fb); int drm_fb_cma_create_handle(struct drm_framebuffer *fb,