Re: [RFC 1/5] drm: Add DRM support for tiny LCD displays

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Apr 01, 2016 at 09:15:45PM +0200, Noralf Trønnes wrote:
> Den 29.03.2016 09:27, skrev Daniel Vetter:
> >I was wondering whether we couldn't avoid the _with_funcs here by setting
> >up fbdefio iff ->dirty is set. Then you could add the generic defio setup
> >code to drm_fbdev_cma_create after helper->fb is allocated, but only if
> >helper->fb->funcs->dirty is set. Makes for a bit less boilerplate.
> >
> >Or did I miss something?
> 
> I don't see how I can avoid drm_fbdev_cma_init_with_funcs():
> 
> static struct drm_framebuffer_funcs tinydrm_fb_cma_funcs = {
>         .destroy        = drm_fb_cma_destroy,
>         .create_handle  = drm_fb_cma_create_handle,
>         .dirty          = tinydrm_fbdev_dirty,
> };
> 
> static int tinydrm_fbdev_create(struct drm_fb_helper *helper,
>                                 struct drm_fb_helper_surface_size *sizes)
> {
>         return drm_fbdev_cma_create_with_funcs(helper, sizes,
> &tinydrm_fb_cma_funcs);
> }
> 
> static const struct drm_fb_helper_funcs tinydrm_fb_helper_funcs = {
>         .fb_probe = tinydrm_fbdev_create,
> };
> 
> int tinydrm_fbdev_init(struct tinydrm_device *tdev)
> {
>         struct drm_device *dev = tdev->base;
>         struct drm_fbdev_cma *cma;
> 
>         cma = drm_fbdev_cma_init_with_funcs(dev, 16,
> dev->mode_config.num_crtc,
> dev->mode_config.num_connector,
> &tinydrm_fb_helper_funcs);
>         if (IS_ERR(cma))
>                 return PTR_ERR(cma);
> 
>         tdev->fbdev_cma = cma;
> 
>         return 0;
> }
> 
> 
> Thanks for your feedback so far Daniel, I quite like the direction this is
> taking. I'll try and implement it in a new version of the patchset.

Yeah I was dense really. One option to avoid most of this might be to add
a framebuffer_create helper function to dev->mode_config->helpers (we
don't yet have a vtable for global helper hooks, so would need to add
that), which instead of the top-level uin32_t -> drm_framebuffer gets a
struct *drm_gem_object[4] array parameter with already decoded buffer
object handles. Drivers could then use that to add their own dirtyfb hooks
and other special sauce to drm_framebuffer, while cma would just use that
hook (if it's set) instead of calling cma_create_fb directly.

So yeah, essentially go back to one of your original proposals. But it
will still not be entirely clean, so whatever you think looks better I'd
say ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel