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

2016-04-12 Thread Daniel Vetter
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


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

2016-04-01 Thread Noralf Trønnes

Den 29.03.2016 09:27, skrev Daniel Vetter:
> On Fri, Mar 25, 2016 at 11:41:44AM +0100, Noralf Trønnes wrote:
>> Den 23.03.2016 18:28, skrev Daniel Vetter:
>>> On Wed, Mar 23, 2016 at 06:07:56PM +0100, Noralf Trønnes wrote:
 Den 18.03.2016 18:47, skrev Daniel Vetter:
> On Thu, Mar 17, 2016 at 10:51:55PM +0100, Noralf Trønnes wrote:
>> Den 16.03.2016 16:11, skrev Daniel Vetter:
>>> On Wed, Mar 16, 2016 at 02:34:15PM +0100, Noralf Trønnes wrote:
 tinydrm provides a very simplified view of DRM for displays that has
 onboard video memory and is connected through a slow bus like SPI/I2C.

 Signed-off-by: Noralf Trønnes 
>>> Yay, it finally happens! I already made a comment on the cover letter
>>> about the fbdev stuff, I think that's the biggest part to split out from
>>> tinydrm here. I'm not entirely sure a detailed code review makes sense
>>> before that part is done (and hey we can start merging already), so 
>>> just a
>>> high level review for now:
 [...]
>>> In the case of tinydrm I think that means we should have a bunch of new
>>> drm helpers, or extensions for existing ones:
>>> - fbdev deferred io support using ->dirtyfb in drm_fb_helper.c.
>> Are you thinking something like this?
>>
>> struct drm_fb_helper_funcs {
>>  int (*dirtyfb)(struct drm_fb_helper *fb_helper,
>> struct drm_clip_rect *clip);
> We already have a dirty_fb function in
> dev->mode_config->funcs->dirty_fb(). This is the official interface native
> drm/kms userspace is supposed to use to flush frontbuffer rendering. The
> xfree86-video-modesetting driver uses it.
 I couldn't find this dirty_fb() function, but I assume you mean
 drm_framebuffer_funcs.dirty().
>>> Yup.
>>>
>> };
>>
>> struct drm_fb_helper {
>>  spinlock_t dirty_lock;
>>  struct drm_clip_rect *dirty_clip;
>> };
> Yeah, this part is needed for the delayed work for the fbdev helper.
> struct work dirty_fb_work; is missing.
 This confuses me.
 If we have this then there's no need for a fb->funcs->dirty() call,
 the driver can just add a work function here instead.

 Possible fb dirty() call chain:
 Calls to drm_fb_helper_sys_* or mmap page writes will schedule
 fb_info->deferred_work. The worker func fb_deferred_io_work() calls
 fb_info->fbdefio->deferred_io().
 Then deferred_io() can call fb_helper->fb->funcs->dirty().

 In my use-case this dirty() function would schedule a delayed_work to run
 immediately since it has already been deferred collecting changes.
 The regular drm side framebuffer dirty() collects damage and schedules
 the same worker to run deferred.

 I don't see an easy way for a driver to set the dirty() function in
 drm_fb_cma_helper apart from doing this:

   struct drm_fbdev_cma {
   struct drm_fb_helperfb_helper;
   struct drm_fb_cma   *fb;
 +int (*dirty)(struct drm_framebuffer *framebuffer,
 + struct drm_file *file_priv, unsigned flags,
 + unsigned color, struct drm_clip_rect *clips,
 + unsigned num_clips);
   };
>>> Well my point is that drm core already has a canonical interface
>>> (drm_framebuffer_funcs.dirty) to flush out rendering. And it's supposed to
>>> be called from process context, and userspace is supposed to batch up
>>> dirty updates.

[...]

>
>>> What I'd like is that the fbdev emulation uses exactly that interface,
>>> without requiring drivers to write any additional fbdev code (like qxl and
>>> udl currently have). Since the drm_framebuffer_funcs.dirty is already
>>> expected to run in process context I think the only bit we need is the
>>> deferred_work you already added in fbdev, so that we can schedule the
>>> driver's ->dirty() function.
>>>
>>> There shouldn't be any need to have another ->dirty() function anywhere
>>> else.
>> I'll try and see if I can explain better with some code.
>> First the drm_fb_helper part:
>>
>> void drm_fb_helper_deferred_io(struct fb_info *info,
>> struct list_head *pagelist)
>> {
>>  struct drm_fb_helper *helper = info->par;
>>  unsigned long start, end, min, max;
>>  struct drm_clip_rect clip;
>>  struct page *page;
>>
>>  if (!helper->fb->funcs->dirty)
>>  return;
>>
>>  spin_lock(&helper->dirty_lock);
>>  clip = *helper->dirty_clip;
>>  /* reset dirty_clip */
>>  spin_unlock(&helper->dirty_lock);
>>
>>  min = ULONG_MAX;
>>  max = 0;
>>  list_for_each_entry(page, pagelist, lru) {
>>  start = page->index << PAGE_SHIFT;
>>  end = start + PAGE_SIZE - 1;
>>  min = min(min, start);
>>  max = max(max, end);
>> 

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

2016-03-29 Thread Daniel Vetter
On Fri, Mar 25, 2016 at 11:41:44AM +0100, Noralf Trønnes wrote:
> 
> Den 23.03.2016 18:28, skrev Daniel Vetter:
> >On Wed, Mar 23, 2016 at 06:07:56PM +0100, Noralf Trønnes wrote:
> >>Den 18.03.2016 18:47, skrev Daniel Vetter:
> >>>On Thu, Mar 17, 2016 at 10:51:55PM +0100, Noralf Trønnes wrote:
> Den 16.03.2016 16:11, skrev Daniel Vetter:
> >On Wed, Mar 16, 2016 at 02:34:15PM +0100, Noralf Trønnes wrote:
> >>tinydrm provides a very simplified view of DRM for displays that has
> >>onboard video memory and is connected through a slow bus like SPI/I2C.
> >>
> >>Signed-off-by: Noralf Trønnes 
> >Yay, it finally happens! I already made a comment on the cover letter
> >about the fbdev stuff, I think that's the biggest part to split out from
> >tinydrm here. I'm not entirely sure a detailed code review makes sense
> >before that part is done (and hey we can start merging already), so just 
> >a
> >high level review for now:
> >>[...]
> >>>
> >In the case of tinydrm I think that means we should have a bunch of new
> >drm helpers, or extensions for existing ones:
> >- fbdev deferred io support using ->dirtyfb in drm_fb_helper.c.
> Are you thinking something like this?
> 
> struct drm_fb_helper_funcs {
>  int (*dirtyfb)(struct drm_fb_helper *fb_helper,
> struct drm_clip_rect *clip);
> >>>We already have a dirty_fb function in
> >>>dev->mode_config->funcs->dirty_fb(). This is the official interface native
> >>>drm/kms userspace is supposed to use to flush frontbuffer rendering. The
> >>>xfree86-video-modesetting driver uses it.
> >>I couldn't find this dirty_fb() function, but I assume you mean
> >>drm_framebuffer_funcs.dirty().
> >Yup.
> >
> };
> 
> struct drm_fb_helper {
>  spinlock_t dirty_lock;
>  struct drm_clip_rect *dirty_clip;
> };
> >>>Yeah, this part is needed for the delayed work for the fbdev helper.
> >>>struct work dirty_fb_work; is missing.
> >>This confuses me.
> >>If we have this then there's no need for a fb->funcs->dirty() call,
> >>the driver can just add a work function here instead.
> >>
> >>Possible fb dirty() call chain:
> >>Calls to drm_fb_helper_sys_* or mmap page writes will schedule
> >>fb_info->deferred_work. The worker func fb_deferred_io_work() calls
> >>fb_info->fbdefio->deferred_io().
> >>Then deferred_io() can call fb_helper->fb->funcs->dirty().
> >>
> >>In my use-case this dirty() function would schedule a delayed_work to run
> >>immediately since it has already been deferred collecting changes.
> >>The regular drm side framebuffer dirty() collects damage and schedules
> >>the same worker to run deferred.
> >>
> >>I don't see an easy way for a driver to set the dirty() function in
> >>drm_fb_cma_helper apart from doing this:
> >>
> >>  struct drm_fbdev_cma {
> >>  struct drm_fb_helperfb_helper;
> >>  struct drm_fb_cma   *fb;
> >>+int (*dirty)(struct drm_framebuffer *framebuffer,
> >>+ struct drm_file *file_priv, unsigned flags,
> >>+ unsigned color, struct drm_clip_rect *clips,
> >>+ unsigned num_clips);
> >>  };
> >Well my point is that drm core already has a canonical interface
> >(drm_framebuffer_funcs.dirty) to flush out rendering. And it's supposed to
> >be called from process context, and userspace is supposed to batch up
> >dirty updates.
> 
> Batched up into one ioctl call?
> If that's the case, then I don't have to use delayed_work like I do now.
> I can just queue a work_struct to run immediately.
> 
> This comment in include/uapi/drm/drm_mode.h made me think that I might
> receive multiple calls:
> 
>  * The kernel or hardware is free to update more then just the
>  * region specified by the clip rects. The kernel or hardware
>  * may also delay and/or coalesce several calls to dirty into a
>  * single update.

This just means that userspace must ensure that the entire buffer is still
valid, specifically that regions outside of the cliprects are not garabge.
This way drivers can increase the area that's uploaded (e.g. upload the
entire screen or only 1 rect spanning all cliprects in case that's the
only thing the hw can do).

Userspace otoh is supposed to batch up updates into one ioctl call (and
not one per drawn glyph or something silly like that).

> I have assumed that I can't run the whole display update from the ioctl
> call since one full display update on the "worst" display takes ~200ms.
> But maybe it's fine to run all this in the ioctl call?

Hm, thus far all drivers do their updating synchronously. But then none
yet took 200ms to do that - most have hw dma for the upload itself.

Short term I'd just do the update synchronously, async dirty with this
slow display has all kinds of fun problems I guess with X rendering
getting ahead and reducing interactivity even more. Long term there's talk
about adding cliprects to atomic, and then we c

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

2016-03-25 Thread Noralf Trønnes

Den 23.03.2016 18:37, skrev David Herrmann:
> Hey
>
> On Wed, Mar 16, 2016 at 2:34 PM, Noralf Trønnes  
> wrote:
>> tinydrm provides a very simplified view of DRM for displays that has
>> onboard video memory and is connected through a slow bus like SPI/I2C.
>>
>> Signed-off-by: Noralf Trønnes 
>> ---

[...]

>> +static struct drm_driver tinydrm_driver = {
>> +   .driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME
>> +   | DRIVER_ATOMIC,
>> +   .load   = tinydrm_load,
>> +   .lastclose  = tinydrm_lastclose,
>> +// .unload = tinydrm_unload,
>> +   .get_vblank_counter = drm_vblank_count,
>> +// .enable_vblank  = tinydrm_enable_vblank,
>> +// .disable_vblank = tinydrm_disable_vblank,
>> +   .gem_free_object= drm_gem_cma_free_object,
>> +   .gem_vm_ops = &drm_gem_cma_vm_ops,
>> +   .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>> +   .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>> +   .gem_prime_import   = drm_gem_prime_import,
>> +   .gem_prime_export   = drm_gem_prime_export,
>> +   .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
>> +   .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
>> +   .gem_prime_vmap = drm_gem_cma_prime_vmap,
>> +   .gem_prime_vunmap   = drm_gem_cma_prime_vunmap,
>> +   .gem_prime_mmap = drm_gem_cma_prime_mmap,
>> +   .dumb_create= drm_gem_cma_dumb_create,
>> +   .dumb_map_offset= drm_gem_cma_dumb_map_offset,
>> +   .dumb_destroy   = drm_gem_dumb_destroy,
>> +   .fops   = &tinydrm_fops,
>> +   .name   = "tinydrm",
>> +   .desc   = "tinydrm",
>> +   .date   = "20150916",
> Can we just drop "date" and "desc" from new drivers? It doesn't add any value.
>
>> +   .major  = 1,
>> +   .minor  = 0,
>> +};
>> +
>> +void tinydrm_release(struct tinydrm_device *tdev)
> We usually prefer "unregister()" to stay consistent with "register()".

Sure.

>> +{
>> +   DRM_DEBUG_KMS("\n");
>> +
>> +   if (tdev->deferred)
>> +   cancel_delayed_work_sync(&tdev->deferred->dwork);
>> +
>> +   tinydrm_fbdev_fini(tdev);
>> +
>> +   drm_panel_detach(&tdev->panel);
>> +   drm_panel_remove(&tdev->panel);
>> +
>> +   drm_mode_config_cleanup(tdev->base);
>> +   drm_dev_unregister(tdev->base);
>> +   drm_dev_unref(tdev->base);
>> +}
>> +EXPORT_SYMBOL(tinydrm_release);
>> +
>> +int tinydrm_register(struct device *dev, struct tinydrm_device *tdev)
>> +{
>> +   struct drm_driver *driver = &tinydrm_driver;
>> +   struct drm_device *ddev;
>> +   int ret;
>> +
>> +   dev_info(dev, "%s\n", __func__);
>> +
>> +dev->coherent_dma_mask = DMA_BIT_MASK(32);
>> +
>> +   if (WARN_ON(!tdev->dirtyfb))
>> +   return -EINVAL;
>> +
>> +   ddev = drm_dev_alloc(driver, dev);
>> +   if (!ddev)
>> +   return -ENOMEM;
>> +
>> +   tdev->base = ddev;
>> +   ddev->dev_private = tdev;
>> +
>> +   ret = drm_dev_set_unique(ddev, dev_name(ddev->dev));
>> +   if (ret)
>> +   goto err_free;
>> +
>> +   ret = drm_dev_register(ddev, 0);
>> +   if (ret)
>> +   goto err_free;
> Whatever your .load() callback does, do that here and drop it. It is
> really not needed. Optionally do it before calling drm_dev_register(),
> depending on which semantics you want.

Ok.

> In general, this looks very similar to what I did with SimpleDRM.

SimpleDRM was the first drm code I studied and tinydrm started with chunks
of code from it :-)

> However, I wonder whether we can make it more modular. Right now it
> always adds code for fbdev, CMA, backlight, etc., but as Daniel
> mentioned those better live in DRM-core helpers.

Yes I will make the next version more modular.
Instead of trying to guess which parts would fit as core helpers, I just
put everything into one module and posted an RFC hoping to get some 
feedback.
I will try an implement the suggestions Daniel has made.

> I'll try forward porting the SimpleDRM drivers to it, and let you know
> whether it works out.
>
> Thanks
> David




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

2016-03-25 Thread Noralf Trønnes

Den 23.03.2016 18:28, skrev Daniel Vetter:
> On Wed, Mar 23, 2016 at 06:07:56PM +0100, Noralf Trønnes wrote:
>> Den 18.03.2016 18:47, skrev Daniel Vetter:
>>> On Thu, Mar 17, 2016 at 10:51:55PM +0100, Noralf Trønnes wrote:
 Den 16.03.2016 16:11, skrev Daniel Vetter:
> On Wed, Mar 16, 2016 at 02:34:15PM +0100, Noralf Trønnes wrote:
>> tinydrm provides a very simplified view of DRM for displays that has
>> onboard video memory and is connected through a slow bus like SPI/I2C.
>>
>> Signed-off-by: Noralf Trønnes 
> Yay, it finally happens! I already made a comment on the cover letter
> about the fbdev stuff, I think that's the biggest part to split out from
> tinydrm here. I'm not entirely sure a detailed code review makes sense
> before that part is done (and hey we can start merging already), so just a
> high level review for now:
>> [...]
>>>
> In the case of tinydrm I think that means we should have a bunch of new
> drm helpers, or extensions for existing ones:
> - fbdev deferred io support using ->dirtyfb in drm_fb_helper.c.
 Are you thinking something like this?

 struct drm_fb_helper_funcs {
  int (*dirtyfb)(struct drm_fb_helper *fb_helper,
 struct drm_clip_rect *clip);
>>> We already have a dirty_fb function in
>>> dev->mode_config->funcs->dirty_fb(). This is the official interface native
>>> drm/kms userspace is supposed to use to flush frontbuffer rendering. The
>>> xfree86-video-modesetting driver uses it.
>> I couldn't find this dirty_fb() function, but I assume you mean
>> drm_framebuffer_funcs.dirty().
> Yup.
>
 };

 struct drm_fb_helper {
  spinlock_t dirty_lock;
  struct drm_clip_rect *dirty_clip;
 };
>>> Yeah, this part is needed for the delayed work for the fbdev helper.
>>> struct work dirty_fb_work; is missing.
>> This confuses me.
>> If we have this then there's no need for a fb->funcs->dirty() call,
>> the driver can just add a work function here instead.
>>
>> Possible fb dirty() call chain:
>> Calls to drm_fb_helper_sys_* or mmap page writes will schedule
>> fb_info->deferred_work. The worker func fb_deferred_io_work() calls
>> fb_info->fbdefio->deferred_io().
>> Then deferred_io() can call fb_helper->fb->funcs->dirty().
>>
>> In my use-case this dirty() function would schedule a delayed_work to run
>> immediately since it has already been deferred collecting changes.
>> The regular drm side framebuffer dirty() collects damage and schedules
>> the same worker to run deferred.
>>
>> I don't see an easy way for a driver to set the dirty() function in
>> drm_fb_cma_helper apart from doing this:
>>
>>   struct drm_fbdev_cma {
>>   struct drm_fb_helperfb_helper;
>>   struct drm_fb_cma   *fb;
>> +int (*dirty)(struct drm_framebuffer *framebuffer,
>> + struct drm_file *file_priv, unsigned flags,
>> + unsigned color, struct drm_clip_rect *clips,
>> + unsigned num_clips);
>>   };
> Well my point is that drm core already has a canonical interface
> (drm_framebuffer_funcs.dirty) to flush out rendering. And it's supposed to
> be called from process context, and userspace is supposed to batch up
> dirty updates.

Batched up into one ioctl call?
If that's the case, then I don't have to use delayed_work like I do now.
I can just queue a work_struct to run immediately.

This comment in include/uapi/drm/drm_mode.h made me think that I might
receive multiple calls:

  * The kernel or hardware is free to update more then just the
  * region specified by the clip rects. The kernel or hardware
  * may also delay and/or coalesce several calls to dirty into a
  * single update.

I have assumed that I can't run the whole display update from the ioctl
call since one full display update on the "worst" display takes ~200ms.
But maybe it's fine to run all this in the ioctl call?

> What I'd like is that the fbdev emulation uses exactly that interface,
> without requiring drivers to write any additional fbdev code (like qxl and
> udl currently have). Since the drm_framebuffer_funcs.dirty is already
> expected to run in process context I think the only bit we need is the
> deferred_work you already added in fbdev, so that we can schedule the
> driver's ->dirty() function.
>
> There shouldn't be any need to have another ->dirty() function anywhere
> else.

I'll try and see if I can explain better with some code.
First the drm_fb_helper part:

void drm_fb_helper_deferred_io(struct fb_info *info,
struct list_head *pagelist)
{
 struct drm_fb_helper *helper = info->par;
 unsigned long start, end, min, max;
 struct drm_clip_rect clip;
 struct page *page;

 if (!helper->fb->funcs->dirty)
 return;

 spin_lock(&helper->dirty_lock);
 clip = *helper->dirty_clip;
 /* reset dirty_clip */
 s

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

2016-03-23 Thread David Herrmann
Hey

On Wed, Mar 16, 2016 at 2:34 PM, Noralf Trønnes  wrote:
> tinydrm provides a very simplified view of DRM for displays that has
> onboard video memory and is connected through a slow bus like SPI/I2C.
>
> Signed-off-by: Noralf Trønnes 
> ---
>  drivers/gpu/drm/Kconfig|   2 +
>  drivers/gpu/drm/Makefile   |   1 +
>  drivers/gpu/drm/tinydrm/Kconfig|  11 +
>  drivers/gpu/drm/tinydrm/Makefile   |   1 +
>  drivers/gpu/drm/tinydrm/core/Makefile  |   8 +
>  drivers/gpu/drm/tinydrm/core/internal.h|  43 +++
>  drivers/gpu/drm/tinydrm/core/tinydrm-core.c| 194 
>  drivers/gpu/drm/tinydrm/core/tinydrm-crtc.c| 203 
>  drivers/gpu/drm/tinydrm/core/tinydrm-deferred.c| 116 +++
>  drivers/gpu/drm/tinydrm/core/tinydrm-fbdev.c   | 345 
> +
>  drivers/gpu/drm/tinydrm/core/tinydrm-framebuffer.c | 112 +++
>  drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c |  97 ++
>  drivers/gpu/drm/tinydrm/core/tinydrm-plane.c   |  50 +++
>  include/drm/tinydrm/tinydrm.h  | 142 +
>  14 files changed, 1325 insertions(+)
>  create mode 100644 drivers/gpu/drm/tinydrm/Kconfig
>  create mode 100644 drivers/gpu/drm/tinydrm/Makefile
>  create mode 100644 drivers/gpu/drm/tinydrm/core/Makefile
>  create mode 100644 drivers/gpu/drm/tinydrm/core/internal.h
>  create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-core.c
>  create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-crtc.c
>  create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-deferred.c
>  create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-fbdev.c
>  create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-framebuffer.c
>  create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
>  create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-plane.c
>  create mode 100644 include/drm/tinydrm/tinydrm.h
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index c4bf9a1..3f8ede0 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -266,3 +266,5 @@ source "drivers/gpu/drm/amd/amdkfd/Kconfig"
>  source "drivers/gpu/drm/imx/Kconfig"
>
>  source "drivers/gpu/drm/vc4/Kconfig"
> +
> +source "drivers/gpu/drm/tinydrm/Kconfig"
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 1e9ff4c..c7c5c16 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -75,3 +75,4 @@ obj-y += i2c/
>  obj-y  += panel/
>  obj-y  += bridge/
>  obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
> +obj-$(CONFIG_DRM_TINYDRM) += tinydrm/
> diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig
> new file mode 100644
> index 000..f290045
> --- /dev/null
> +++ b/drivers/gpu/drm/tinydrm/Kconfig
> @@ -0,0 +1,11 @@
> +menuconfig DRM_TINYDRM
> +   tristate "Support for small TFT LCD display modules"
> +   depends on DRM
> +   select DRM_KMS_HELPER
> +   select DRM_KMS_CMA_HELPER
> +   select DRM_GEM_CMA_HELPER
> +   select DRM_PANEL
> +   select VIDEOMODE_HELPERS
> +   help
> + Choose this option if you have a tinydrm supported display.
> + If M is selected the module will be called tinydrm.
> diff --git a/drivers/gpu/drm/tinydrm/Makefile 
> b/drivers/gpu/drm/tinydrm/Makefile
> new file mode 100644
> index 000..7476ed1
> --- /dev/null
> +++ b/drivers/gpu/drm/tinydrm/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_DRM_TINYDRM)  += core/
> diff --git a/drivers/gpu/drm/tinydrm/core/Makefile 
> b/drivers/gpu/drm/tinydrm/core/Makefile
> new file mode 100644
> index 000..03309f4
> --- /dev/null
> +++ b/drivers/gpu/drm/tinydrm/core/Makefile
> @@ -0,0 +1,8 @@
> +obj-$(CONFIG_DRM_TINYDRM)  += tinydrm.o
> +tinydrm-y  += tinydrm-core.o
> +tinydrm-y  += tinydrm-crtc.o
> +tinydrm-y  += tinydrm-framebuffer.o
> +tinydrm-y  += tinydrm-plane.o
> +tinydrm-y  += tinydrm-helpers.o
> +tinydrm-y  += tinydrm-deferred.o
> +tinydrm-$(CONFIG_DRM_KMS_FB_HELPER)+= tinydrm-fbdev.o
> diff --git a/drivers/gpu/drm/tinydrm/core/internal.h 
> b/drivers/gpu/drm/tinydrm/core/internal.h
> new file mode 100644
> index 000..a126658
> --- /dev/null
> +++ b/drivers/gpu/drm/tinydrm/core/internal.h
> @@ -0,0 +1,43 @@
> +/*
> + * Copyright (C) 2016 Noralf Trønnes
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +int tinydrm_crtc_create(struct tinydrm_device *tdev);
> +
> +static inline bool tinydrm_active(struct tinydrm

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

2016-03-23 Thread Daniel Vetter
On Wed, Mar 23, 2016 at 06:07:56PM +0100, Noralf Trønnes wrote:
> 
> Den 18.03.2016 18:47, skrev Daniel Vetter:
> >On Thu, Mar 17, 2016 at 10:51:55PM +0100, Noralf Trønnes wrote:
> >>Den 16.03.2016 16:11, skrev Daniel Vetter:
> >>>On Wed, Mar 16, 2016 at 02:34:15PM +0100, Noralf Trønnes wrote:
> tinydrm provides a very simplified view of DRM for displays that has
> onboard video memory and is connected through a slow bus like SPI/I2C.
> 
> Signed-off-by: Noralf Trønnes 
> >>>Yay, it finally happens! I already made a comment on the cover letter
> >>>about the fbdev stuff, I think that's the biggest part to split out from
> >>>tinydrm here. I'm not entirely sure a detailed code review makes sense
> >>>before that part is done (and hey we can start merging already), so just a
> >>>high level review for now:
> [...]
> >
> >>>In the case of tinydrm I think that means we should have a bunch of new
> >>>drm helpers, or extensions for existing ones:
> >>>- fbdev deferred io support using ->dirtyfb in drm_fb_helper.c.
> >>Are you thinking something like this?
> >>
> >>struct drm_fb_helper_funcs {
> >> int (*dirtyfb)(struct drm_fb_helper *fb_helper,
> >>struct drm_clip_rect *clip);
> >We already have a dirty_fb function in
> >dev->mode_config->funcs->dirty_fb(). This is the official interface native
> >drm/kms userspace is supposed to use to flush frontbuffer rendering. The
> >xfree86-video-modesetting driver uses it.
> 
> I couldn't find this dirty_fb() function, but I assume you mean
> drm_framebuffer_funcs.dirty().

Yup.

> >>};
> >>
> >>struct drm_fb_helper {
> >> spinlock_t dirty_lock;
> >> struct drm_clip_rect *dirty_clip;
> >>};
> >Yeah, this part is needed for the delayed work for the fbdev helper.
> 
> >struct work dirty_fb_work; is missing.
> 
> This confuses me.
> If we have this then there's no need for a fb->funcs->dirty() call,
> the driver can just add a work function here instead.
> 
> Possible fb dirty() call chain:
> Calls to drm_fb_helper_sys_* or mmap page writes will schedule
> fb_info->deferred_work. The worker func fb_deferred_io_work() calls
> fb_info->fbdefio->deferred_io().
> Then deferred_io() can call fb_helper->fb->funcs->dirty().
> 
> In my use-case this dirty() function would schedule a delayed_work to run
> immediately since it has already been deferred collecting changes.
> The regular drm side framebuffer dirty() collects damage and schedules
> the same worker to run deferred.
> 
> I don't see an easy way for a driver to set the dirty() function in
> drm_fb_cma_helper apart from doing this:
> 
>  struct drm_fbdev_cma {
>  struct drm_fb_helperfb_helper;
>  struct drm_fb_cma   *fb;
> +int (*dirty)(struct drm_framebuffer *framebuffer,
> + struct drm_file *file_priv, unsigned flags,
> + unsigned color, struct drm_clip_rect *clips,
> + unsigned num_clips);
>  };

Well my point is that drm core already has a canonical interface
(drm_framebuffer_funcs.dirty) to flush out rendering. And it's supposed to
be called from process context, and userspace is supposed to batch up
dirty updates.

What I'd like is that the fbdev emulation uses exactly that interface,
without requiring drivers to write any additional fbdev code (like qxl and
udl currently have). Since the drm_framebuffer_funcs.dirty is already
expected to run in process context I think the only bit we need is the
deferred_work you already added in fbdev, so that we can schedule the
driver's ->dirty() function.

There shouldn't be any need to have another ->dirty() function anywhere
else.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


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

2016-03-23 Thread Noralf Trønnes

Den 18.03.2016 18:47, skrev Daniel Vetter:
> On Thu, Mar 17, 2016 at 10:51:55PM +0100, Noralf Trønnes wrote:
>> Den 16.03.2016 16:11, skrev Daniel Vetter:
>>> On Wed, Mar 16, 2016 at 02:34:15PM +0100, Noralf Trønnes wrote:
 tinydrm provides a very simplified view of DRM for displays that has
 onboard video memory and is connected through a slow bus like SPI/I2C.

 Signed-off-by: Noralf Trønnes 
>>> Yay, it finally happens! I already made a comment on the cover letter
>>> about the fbdev stuff, I think that's the biggest part to split out from
>>> tinydrm here. I'm not entirely sure a detailed code review makes sense
>>> before that part is done (and hey we can start merging already), so just a
>>> high level review for now:
[...]
>
>>> In the case of tinydrm I think that means we should have a bunch of new
>>> drm helpers, or extensions for existing ones:
>>> - fbdev deferred io support using ->dirtyfb in drm_fb_helper.c.
>> Are you thinking something like this?
>>
>> struct drm_fb_helper_funcs {
>>  int (*dirtyfb)(struct drm_fb_helper *fb_helper,
>> struct drm_clip_rect *clip);
> We already have a dirty_fb function in
> dev->mode_config->funcs->dirty_fb(). This is the official interface native
> drm/kms userspace is supposed to use to flush frontbuffer rendering. The
> xfree86-video-modesetting driver uses it.

I couldn't find this dirty_fb() function, but I assume you mean
drm_framebuffer_funcs.dirty().

>> };
>>
>> struct drm_fb_helper {
>>  spinlock_t dirty_lock;
>>  struct drm_clip_rect *dirty_clip;
>> };
> Yeah, this part is needed for the delayed work for the fbdev helper.

> struct work dirty_fb_work; is missing.

This confuses me.
If we have this then there's no need for a fb->funcs->dirty() call,
the driver can just add a work function here instead.

Possible fb dirty() call chain:
Calls to drm_fb_helper_sys_* or mmap page writes will schedule
fb_info->deferred_work. The worker func fb_deferred_io_work() calls
fb_info->fbdefio->deferred_io().
Then deferred_io() can call fb_helper->fb->funcs->dirty().

In my use-case this dirty() function would schedule a delayed_work to run
immediately since it has already been deferred collecting changes.
The regular drm side framebuffer dirty() collects damage and schedules
the same worker to run deferred.

I don't see an easy way for a driver to set the dirty() function in
drm_fb_cma_helper apart from doing this:

  struct drm_fbdev_cma {
  struct drm_fb_helperfb_helper;
  struct drm_fb_cma   *fb;
+int (*dirty)(struct drm_framebuffer *framebuffer,
+ struct drm_file *file_priv, unsigned flags,
+ unsigned color, struct drm_clip_rect *clips,
+ unsigned num_clips);
  };


>> Initially I used drm_fb_cma_helper.c with some added deferred code.
>> This worked fine for fbcon, but the deferred mmap part didn't work well.
>> For instance when using fbtest, I got short random horizontal lines on the
>> display that didn't contain the latest pixels. I had to write several times
>> to /dev/fb1 to trigger a display update to get all the previous pixels to go
>> away and get the current image. Maybe it's some caching issue, I don't know.
>> The Raspberry Pi doesn't support 16-bit SPI, so tinydrm does a byte swap to
>> a new buffer before sending it using 8-bit.
>> Maybe I need to call some kind of DMA sync function?
> drm_fb_cma_helper is for creating drm_framebuffer backed by cma allocator
> objects. How you create drm_framebuffer is orthogonal to whether you have
> a ->dirty_fb hook (and hence needed defio support in fbdev) or not. E.g.
> maybe some SPI device has a dma engine, and hence you want to allocate
> drm_framebuffer using cma. On others with an i2c bus you want to just
> allocate kernel memory, since the cpu will copy the data anyway.
>
> That's why I think we need to make sure this split is still maintained.
>
>> The dumb buffer uses drm_gem_cma_dumb_create() which is backed by cma, and
>> that works just fine (I have only tested with David Herrmann's modeset[1]).
>> A similar byte swapping happens here.
>>
>> I also had to do this for the deferred io to work:
>>
>> info->fix.smem_start = __pa(info->screen_base);
>>
>> drm_fb_cma_helper assigns the dma address to smem_start, but at least on
>> the Raspberry Pi this bus address can't be used by deferred_io
>> (fb_deferred_io_fault()). And the ARM version of __pa states that it
>> shouldn't be used by drivers, so when my vmalloc version worked, I went
>> with that. But I see that there's a virt_to_phys() function that doesn't
>> have that statement about not being used by drivers, so maybe this isn't
>> a show stopper after all?
>>
>> Any thoughts on this problem? I would rather have a cma backed fbdev
>> framebuffer since that would give me the same type of memory both for
>> fbdev and DRM.
> Hm, tbh I have no clear idea who fbdev fb memory mapping workings. The
> above comme

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

2016-03-18 Thread Daniel Vetter
On Thu, Mar 17, 2016 at 10:51:55PM +0100, Noralf Trønnes wrote:
> 
> Den 16.03.2016 16:11, skrev Daniel Vetter:
> >On Wed, Mar 16, 2016 at 02:34:15PM +0100, Noralf Trønnes wrote:
> >>tinydrm provides a very simplified view of DRM for displays that has
> >>onboard video memory and is connected through a slow bus like SPI/I2C.
> >>
> >>Signed-off-by: Noralf Trønnes 
> >Yay, it finally happens! I already made a comment on the cover letter
> >about the fbdev stuff, I think that's the biggest part to split out from
> >tinydrm here. I'm not entirely sure a detailed code review makes sense
> >before that part is done (and hey we can start merging already), so just a
> >high level review for now:
> >
> >The big story in kms/drm in the past years is that we've rejecting
> >anything that remotely looks like a midlayer. Instead the preferred design
> >pattern is a library of helper functions to implement useful default
> >behaviour, or sometimes just building blocks for useful default behaviour.
> >And then build up real drivers using these pieces. The benefit of that is
> >two-fold:
> >- easier to share code with other drivers that only need part of the
> >   behaviour (e.g. fbdev deferred io support).
> >- easier to adapt to special hw that needs exceptions since worst case you
> >   can just copypaste an entire hook. Or implement the special case and
> >   call the default helper for the normal cases.
> >
> >lwn has a good article on this pattern:
> >
> >https://lwn.net/Articles/336262/
> 
> I was afraid you would say "midlayer" :-)
> 
> How about creating macros like SIMPLE_DEV_PM_OPS and friends to simplify
> the drm_driver boilerplate and use that in the drivers?
> 
> #define SET_DRM_DRIVER_GEM_CMA_OPS \
> .gem_free_object= drm_gem_cma_free_object, \
> .gem_vm_ops= &drm_gem_cma_vm_ops, \
> .prime_handle_to_fd= drm_gem_prime_handle_to_fd, \
> .prime_fd_to_handle= drm_gem_prime_fd_to_handle, \
> .gem_prime_import= drm_gem_prime_import, \
> .gem_prime_export= drm_gem_prime_export, \
> .gem_prime_get_sg_table= drm_gem_cma_prime_get_sg_table, \
> .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, \
> .gem_prime_vmap= drm_gem_cma_prime_vmap, \
> .gem_prime_vunmap= drm_gem_cma_prime_vunmap, \
> .gem_prime_mmap= drm_gem_cma_prime_mmap, \
> .dumb_create= drm_gem_cma_dumb_create, \
> .dumb_map_offset= drm_gem_cma_dumb_map_offset, \
> .dumb_destroy= drm_gem_dumb_destroy,
> 
> #define TINYDRM_DRM_DRIVER(name_struct, name_str, desc_str, date_str) \
> static struct drm_driver name_struct = { \
> .driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME \
> | DRIVER_ATOMIC, \
> .load= tinydrm_load, \
> .unload= tinydrm_unload, \
> .lastclose= tinydrm_lastclose, \
> SET_DRM_DRIVER_GEM_CMA_OPS \
> .fops= &tinydrm_fops, \
> .name= name_str, \
> .desc= desc_str, \
> .date= date_str, \
> .major= 1, \
> .minor= 0, \
> }

Looks like a pretty sweet idea. Maybe even split it up into GEM_PRIME_OPS
and similar sub-groups, which you could roll out into lots of drivers.

> 
> Now the driver can do this:
> TINYDRM_DRM_DRIVER(adafruit_tft, "adafruit-tft", Adafruit TFT", "20160317");
> 
> In addition to that, the tinydrm specific parts that make up
> tinydrm_load/unload can be exported if someone wants it slightly different.

Just from your sketch here this sounds nifty.

> >In the case of tinydrm I think that means we should have a bunch of new
> >drm helpers, or extensions for existing ones:
> >- fbdev deferred io support using ->dirtyfb in drm_fb_helper.c.
> 
> Are you thinking something like this?
> 
> struct drm_fb_helper_funcs {
> int (*dirtyfb)(struct drm_fb_helper *fb_helper,
>struct drm_clip_rect *clip);

We already have a dirty_fb function in
dev->mode_config->funcs->dirty_fb(). This is the official interface native
drm/kms userspace is supposed to use to flush frontbuffer rendering. The
xfree86-video-modesetting driver uses it.

> };
> 
> struct drm_fb_helper {
> spinlock_t dirty_lock;
> struct drm_clip_rect *dirty_clip;
> };

Yeah, this part is needed for the delayed work for the fbdev helper.
struct work dirty_fb_work; is missing.

> Should I extend drm_fb_helper_sys_* or make it explicit with
> drm_fb_helper_sys_*_deferred functions?

Imo extend the existing helpers, adding deferred io is part of the reasons
we added them. Just add a check for mode_config->funcs->dirty_fb to
short-circuit all the deferred io flushing if the driver doesn't need it.

Same for setting up deferred io fbdev driver flags - just check whether
->dirty_fb is there, and only if that is set fill in inof->fbdefio.

> #ifdef CONFIG_FB_DEFERRED_IO
> /* Will just return if info->fbdefio is not set */
> void drm_fb_helper_s

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

2016-03-17 Thread Noralf Trønnes

Den 16.03.2016 16:11, skrev Daniel Vetter:
> On Wed, Mar 16, 2016 at 02:34:15PM +0100, Noralf Trønnes wrote:
>> tinydrm provides a very simplified view of DRM for displays that has
>> onboard video memory and is connected through a slow bus like SPI/I2C.
>>
>> Signed-off-by: Noralf Trønnes 
> Yay, it finally happens! I already made a comment on the cover letter
> about the fbdev stuff, I think that's the biggest part to split out from
> tinydrm here. I'm not entirely sure a detailed code review makes sense
> before that part is done (and hey we can start merging already), so just a
> high level review for now:
>
> The big story in kms/drm in the past years is that we've rejecting
> anything that remotely looks like a midlayer. Instead the preferred design
> pattern is a library of helper functions to implement useful default
> behaviour, or sometimes just building blocks for useful default behaviour.
> And then build up real drivers using these pieces. The benefit of that is
> two-fold:
> - easier to share code with other drivers that only need part of the
>behaviour (e.g. fbdev deferred io support).
> - easier to adapt to special hw that needs exceptions since worst case you
>can just copypaste an entire hook. Or implement the special case and
>call the default helper for the normal cases.
>
> lwn has a good article on this pattern:
>
> https://lwn.net/Articles/336262/

I was afraid you would say "midlayer" :-)

How about creating macros like SIMPLE_DEV_PM_OPS and friends to simplify
the drm_driver boilerplate and use that in the drivers?

#define SET_DRM_DRIVER_GEM_CMA_OPS \
 .gem_free_object= drm_gem_cma_free_object, \
 .gem_vm_ops= &drm_gem_cma_vm_ops, \
 .prime_handle_to_fd= drm_gem_prime_handle_to_fd, \
 .prime_fd_to_handle= drm_gem_prime_fd_to_handle, \
 .gem_prime_import= drm_gem_prime_import, \
 .gem_prime_export= drm_gem_prime_export, \
 .gem_prime_get_sg_table= drm_gem_cma_prime_get_sg_table, \
 .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, \
 .gem_prime_vmap= drm_gem_cma_prime_vmap, \
 .gem_prime_vunmap= drm_gem_cma_prime_vunmap, \
 .gem_prime_mmap= drm_gem_cma_prime_mmap, \
 .dumb_create= drm_gem_cma_dumb_create, \
 .dumb_map_offset= drm_gem_cma_dumb_map_offset, \
 .dumb_destroy= drm_gem_dumb_destroy,

#define TINYDRM_DRM_DRIVER(name_struct, name_str, desc_str, date_str) \
static struct drm_driver name_struct = { \
 .driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME \
 | DRIVER_ATOMIC, \
 .load= tinydrm_load, \
 .unload= tinydrm_unload, \
 .lastclose= tinydrm_lastclose, \
 SET_DRM_DRIVER_GEM_CMA_OPS \
 .fops= &tinydrm_fops, \
 .name= name_str, \
 .desc= desc_str, \
 .date= date_str, \
 .major= 1, \
 .minor= 0, \
}

Now the driver can do this:
TINYDRM_DRM_DRIVER(adafruit_tft, "adafruit-tft", Adafruit TFT", "20160317");

In addition to that, the tinydrm specific parts that make up
tinydrm_load/unload can be exported if someone wants it slightly different.


> In the case of tinydrm I think that means we should have a bunch of new
> drm helpers, or extensions for existing ones:
> - fbdev deferred io support using ->dirtyfb in drm_fb_helper.c.

Are you thinking something like this?

struct drm_fb_helper_funcs {
 int (*dirtyfb)(struct drm_fb_helper *fb_helper,
struct drm_clip_rect *clip);
};

struct drm_fb_helper {
 spinlock_t dirty_lock;
 struct drm_clip_rect *dirty_clip;
};


Should I extend drm_fb_helper_sys_* or make it explicit with
drm_fb_helper_sys_*_deferred functions?

#ifdef CONFIG_FB_DEFERRED_IO
/* Will just return if info->fbdefio is not set */
void drm_fb_helper_sys_deferred(struct fb_info *info, u32 x, u32 y,
 u32 width, u32 height);
#else
static inline void drm_fb_helper_sys_deferred(struct fb_info *info, u32 
x, u32 y,
   u32 width, u32 height)
{ }
#endif

void drm_fb_helper_sys_imageblit(struct fb_info *info,
  const struct fb_image *image)
{
 sys_imageblit(info, image);
 drm_fb_helper_sys_deferred(info, image->dx, image->dy, image->width,
image->height);
}

OR

void drm_fb_helper_sys_imageblit_deferred(struct fb_info *info,
   const struct fb_image *image)
{
 drm_fb_helper_sys_imageblit(info, image);
 drm_fb_helper_sys_deferred(info, image->dx, image->dy, image->width,
image->height);
}


Initially I used drm_fb_cma_helper.c with some added deferred code.
This worked fine for fbcon, but the deferred mmap part didn't work well.
For instance when using fbtest, I got short random horizontal lines on the
display that didn't contain the latest pixels. I had

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

2016-03-16 Thread Daniel Vetter
On Wed, Mar 16, 2016 at 02:34:15PM +0100, Noralf Trønnes wrote:
> tinydrm provides a very simplified view of DRM for displays that has
> onboard video memory and is connected through a slow bus like SPI/I2C.
> 
> Signed-off-by: Noralf Trønnes 

Yay, it finally happens! I already made a comment on the cover letter
about the fbdev stuff, I think that's the biggest part to split out from
tinydrm here. I'm not entirely sure a detailed code review makes sense
before that part is done (and hey we can start merging already), so just a
high level review for now:

The big story in kms/drm in the past years is that we've rejecting
anything that remotely looks like a midlayer. Instead the preferred design
pattern is a library of helper functions to implement useful default
behaviour, or sometimes just building blocks for useful default behaviour.
And then build up real drivers using these pieces. The benefit of that is
two-fold:
- easier to share code with other drivers that only need part of the
  behaviour (e.g. fbdev deferred io support).
- easier to adapt to special hw that needs exceptions since worst case you
  can just copypaste an entire hook. Or implement the special case and
  call the default helper for the normal cases.

lwn has a good article on this pattern:

https://lwn.net/Articles/336262/

In the case of tinydrm I think that means we should have a bunch of new
drm helpers, or extensions for existing ones:
- fbdev deferred io support using ->dirtyfb in drm_fb_helper.c.
- Helper to generate a simple display pipeline with 1 plane, 1 crtc, 1
  encoder pointing at a specific drm_connector. There's lots of other
  simple hw that could use this. Maybe create a new
  drm_simple_kms_helper.c for this.
- A helper to create a simple drm_connector from a drm_panel (the
  get_modes hooks you have here), maybe also in drm_simple_kms_helper.c.
- Helpers to handle dirtyfb, like the clip rect merge function you have in
  here.
- Helper maybe for purely software framebuffers that are uploaded using
  cpu access instead of dma.

Then bus-specific support you have in later patches for tinydrm would each
be a separate drm driver, assemebled using those helper blocks. It's a
notch more boilerplate maybe, but all the separate pieces I listed above
would be useful in drivers outside of just tinydrm. And maybe other
drivers would need almost everything, except the drm_framebuffer must be
alloced using the dma api (i.e. those should use the cma helpers), e.g.
when your dumb bus can do dma of some sort.

Anyway first thoughts, I'm really happy that something like this finally
happens!

Cheers, Daniel
> ---
>  drivers/gpu/drm/Kconfig|   2 +
>  drivers/gpu/drm/Makefile   |   1 +
>  drivers/gpu/drm/tinydrm/Kconfig|  11 +
>  drivers/gpu/drm/tinydrm/Makefile   |   1 +
>  drivers/gpu/drm/tinydrm/core/Makefile  |   8 +
>  drivers/gpu/drm/tinydrm/core/internal.h|  43 +++
>  drivers/gpu/drm/tinydrm/core/tinydrm-core.c| 194 
>  drivers/gpu/drm/tinydrm/core/tinydrm-crtc.c| 203 
>  drivers/gpu/drm/tinydrm/core/tinydrm-deferred.c| 116 +++
>  drivers/gpu/drm/tinydrm/core/tinydrm-fbdev.c   | 345 
> +
>  drivers/gpu/drm/tinydrm/core/tinydrm-framebuffer.c | 112 +++
>  drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c |  97 ++
>  drivers/gpu/drm/tinydrm/core/tinydrm-plane.c   |  50 +++
>  include/drm/tinydrm/tinydrm.h  | 142 +
>  14 files changed, 1325 insertions(+)
>  create mode 100644 drivers/gpu/drm/tinydrm/Kconfig
>  create mode 100644 drivers/gpu/drm/tinydrm/Makefile
>  create mode 100644 drivers/gpu/drm/tinydrm/core/Makefile
>  create mode 100644 drivers/gpu/drm/tinydrm/core/internal.h
>  create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-core.c
>  create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-crtc.c
>  create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-deferred.c
>  create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-fbdev.c
>  create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-framebuffer.c
>  create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
>  create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-plane.c
>  create mode 100644 include/drm/tinydrm/tinydrm.h
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index c4bf9a1..3f8ede0 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -266,3 +266,5 @@ source "drivers/gpu/drm/amd/amdkfd/Kconfig"
>  source "drivers/gpu/drm/imx/Kconfig"
>  
>  source "drivers/gpu/drm/vc4/Kconfig"
> +
> +source "drivers/gpu/drm/tinydrm/Kconfig"
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 1e9ff4c..c7c5c16 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -75,3 +75,4 @@ obj-y   += i2c/
>  obj-y+= pan

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

2016-03-16 Thread Noralf Trønnes
tinydrm provides a very simplified view of DRM for displays that has
onboard video memory and is connected through a slow bus like SPI/I2C.

Signed-off-by: Noralf Trønnes 
---
 drivers/gpu/drm/Kconfig|   2 +
 drivers/gpu/drm/Makefile   |   1 +
 drivers/gpu/drm/tinydrm/Kconfig|  11 +
 drivers/gpu/drm/tinydrm/Makefile   |   1 +
 drivers/gpu/drm/tinydrm/core/Makefile  |   8 +
 drivers/gpu/drm/tinydrm/core/internal.h|  43 +++
 drivers/gpu/drm/tinydrm/core/tinydrm-core.c| 194 
 drivers/gpu/drm/tinydrm/core/tinydrm-crtc.c| 203 
 drivers/gpu/drm/tinydrm/core/tinydrm-deferred.c| 116 +++
 drivers/gpu/drm/tinydrm/core/tinydrm-fbdev.c   | 345 +
 drivers/gpu/drm/tinydrm/core/tinydrm-framebuffer.c | 112 +++
 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c |  97 ++
 drivers/gpu/drm/tinydrm/core/tinydrm-plane.c   |  50 +++
 include/drm/tinydrm/tinydrm.h  | 142 +
 14 files changed, 1325 insertions(+)
 create mode 100644 drivers/gpu/drm/tinydrm/Kconfig
 create mode 100644 drivers/gpu/drm/tinydrm/Makefile
 create mode 100644 drivers/gpu/drm/tinydrm/core/Makefile
 create mode 100644 drivers/gpu/drm/tinydrm/core/internal.h
 create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-core.c
 create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-crtc.c
 create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-deferred.c
 create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-fbdev.c
 create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-framebuffer.c
 create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
 create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-plane.c
 create mode 100644 include/drm/tinydrm/tinydrm.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index c4bf9a1..3f8ede0 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -266,3 +266,5 @@ source "drivers/gpu/drm/amd/amdkfd/Kconfig"
 source "drivers/gpu/drm/imx/Kconfig"

 source "drivers/gpu/drm/vc4/Kconfig"
+
+source "drivers/gpu/drm/tinydrm/Kconfig"
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 1e9ff4c..c7c5c16 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -75,3 +75,4 @@ obj-y += i2c/
 obj-y  += panel/
 obj-y  += bridge/
 obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
+obj-$(CONFIG_DRM_TINYDRM) += tinydrm/
diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig
new file mode 100644
index 000..f290045
--- /dev/null
+++ b/drivers/gpu/drm/tinydrm/Kconfig
@@ -0,0 +1,11 @@
+menuconfig DRM_TINYDRM
+   tristate "Support for small TFT LCD display modules"
+   depends on DRM
+   select DRM_KMS_HELPER
+   select DRM_KMS_CMA_HELPER
+   select DRM_GEM_CMA_HELPER
+   select DRM_PANEL
+   select VIDEOMODE_HELPERS
+   help
+ Choose this option if you have a tinydrm supported display.
+ If M is selected the module will be called tinydrm.
diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile
new file mode 100644
index 000..7476ed1
--- /dev/null
+++ b/drivers/gpu/drm/tinydrm/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_DRM_TINYDRM)  += core/
diff --git a/drivers/gpu/drm/tinydrm/core/Makefile 
b/drivers/gpu/drm/tinydrm/core/Makefile
new file mode 100644
index 000..03309f4
--- /dev/null
+++ b/drivers/gpu/drm/tinydrm/core/Makefile
@@ -0,0 +1,8 @@
+obj-$(CONFIG_DRM_TINYDRM)  += tinydrm.o
+tinydrm-y  += tinydrm-core.o
+tinydrm-y  += tinydrm-crtc.o
+tinydrm-y  += tinydrm-framebuffer.o
+tinydrm-y  += tinydrm-plane.o
+tinydrm-y  += tinydrm-helpers.o
+tinydrm-y  += tinydrm-deferred.o
+tinydrm-$(CONFIG_DRM_KMS_FB_HELPER)+= tinydrm-fbdev.o
diff --git a/drivers/gpu/drm/tinydrm/core/internal.h 
b/drivers/gpu/drm/tinydrm/core/internal.h
new file mode 100644
index 000..a126658
--- /dev/null
+++ b/drivers/gpu/drm/tinydrm/core/internal.h
@@ -0,0 +1,43 @@
+/*
+ * Copyright (C) 2016 Noralf Trønnes
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+int tinydrm_crtc_create(struct tinydrm_device *tdev);
+
+static inline bool tinydrm_active(struct tinydrm_device *tdev)
+{
+   struct drm_crtc *crtc;
+
+   drm_for_each_crtc(crtc, tdev->base)
+   return crtc->state && crtc->state->active;
+
+   return false;
+}
+
+void tinydrm_mode_config_init(struct tinydrm_device *tdev);
+
+int tinydrm_plane_init(struct tinydrm_