Re: [RFC v3 09/12] drm: Add API for in-kernel clients

[Daniel Vetter]

On Mon, Mar 12, 2018 at 09:21:40PM +0100, Noralf Tr??nnes wrote:
> 
> Den 12.03.2018 17.51, skrev Daniel Vetter:
> > On Thu, Mar 08, 2018 at 06:12:11PM +0100, Noralf Tr??nnes wrote:
> > > Den 06.03.2018 09.56, skrev Daniel Vetter:
> > > > On Thu, Feb 22, 2018 at 09:06:50PM +0100, Noralf Tr??nnes wrote:
> > > > > This adds an API for writing in-kernel clients.
> > > > > 
> > > > > TODO:
> > > > > - Flesh out and complete documentation.
> > > > > - Cloned displays is not tested.
> > > > > - Complete tiled display support and test it.
> > > > > - Test plug/unplug different monitors.
> > > > > - A runtime knob to prevent clients from attaching for debugging 
> > > > > purposes.
> > > > > - Maybe a way to unbind individual client instances.
> > > > > - Maybe take the sysrq support in drm_fb_helper and move it here 
> > > > > somehow.
> > > > > - Add suspend/resume callbacks.
> > > > > Does anyone know why fbdev requires suspend/resume?
> > > > > 
> > > > > Signed-off-by: Noralf Tr??nnes 
> > > > The core client api I like. Some of the opens I'm seeing:
> > > > 
> > > > - If we go with using the internal kms api directly instead of IOCTL
> > > > wrappers then a huge pile of the functions you have here aren't 
> > > > needed
> > > > (e.g. all the event stuff we can just directly use vblank events 
> > > > instead
> > > > of all the wrapping). I'm leaning ever more into that direction, 
> > > > since
> > > > much less code to add.
> > > Looking at drm_fb_helper once again I now see an opportunity to simplify
> > > the modesetting code by nuking drm_fb_helper_connector and stop
> > > maintaining an array of connectors. It looks to be possible to just
> > > create an array temporarily in drm_setup_crtcs() for the duration of the
> > > function. The connectors we care about are ref counted and attached to
> > > modesets. This would remove the need for 
> > > drm_fb_helper_add_one_connector().
> > > 
> > > So I might be able to do struct drm_fb_helper_crtc -> drm_client_crtc
> > > and let the client API take over drm_setup_crtcs(). I'll give it a try.
> > I'm more wondering why we need drm_client_crtc at all, why is drm_crtc not
> > good enough. Or maybe I'm missing something. Imo ioctl wrappers should be
> > the exception where we really, really need them (because the backend of
> > the ioctl isn't implemented in a generic way, e.g. dumb buffers), not for
> > stuff where we already have a perfectly useable in-kernel abi (anything
> > related to modesetting).
> 
> I was talking about moving the modesetting code from drm_fb_helper.c
> to drm_client.c, which meant moving 'struct drm_fb_helper_crtc' as well.
> 
> struct drm_fb_helper_crtc {
>     struct drm_mode_set mode_set;
>     struct drm_display_mode *desired_mode;
>     int x, y;
> };
> 
> But maybe we can get rid of that struct as well.
> The info it contains is also available in drm_mode_set:

drm_mode_set is the uapi struct for the legacy ioctl, but yeah I guess we
can recycle that. And I didn't understand what you wanted to do,
extracting the fbdev helper might make sense. But most of the code seems
fbdev specific, so not sure how much use that would be in other clients.

There's also a bit the problem that it's crufty (it started out as
something that was fully integrated into drivers and digging around in
driver internals directly, hence also the convoluted allocation logic).
-Daniel

> 
> static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
>                 u32 width, u32 height)
> {
> ...
>     drm_fb_helper_for_each_connector(fb_helper, i) {
>         struct drm_display_mode *mode = modes[i];
>         struct drm_fb_helper_crtc *fb_crtc = crtcs[i];
>         struct drm_fb_offset *offset = [i];
> 
>         if (mode && fb_crtc) {
>             struct drm_mode_set *modeset = _crtc->mode_set;
> 
>             fb_crtc->desired_mode = mode;
>             fb_crtc->x = offset->x;
>             fb_crtc->y = offset->y;
>             modeset->mode = drm_mode_duplicate(dev,
>                                fb_crtc->desired_mode);
>             modeset->x = offset->x;
>             modeset->y = offset->y;
>         }
>     }
> 
> I took the hint about my ioctl wrappers :-)
> 
> Noralf.
> 
> > 
> > And in a way the ioctl wrappers wouldn't really be ioctl wrappers
> > conceptually, but simple share some of the same code with the ioctl call
> > chain. The idea is to provide some minimal wrappar around the ->dumb*
> > callbacks.
> > 
> > Anything else is not needed, I think.
> > 
> > > There is one challenge I see upfront and that's the i915 fb_helper
> > > callback in drm_setup_crtcs().
> > > 
> > > > - The register/unregister model needs more thought. Allowing both 
> > > > clients
> > > > to register whenever they want to, and drm_device instances to come 
> > > > and
> > > > go is what fbcon has done, and the resulting locking is a horror 
> > > > show.
> > > > 
> > > > I think if we require that all 

Re: [RFC v3 09/12] drm: Add API for in-kernel clients

[Noralf Trønnes]

Den 12.03.2018 17.51, skrev Daniel Vetter:

On Thu, Mar 08, 2018 at 06:12:11PM +0100, Noralf Tr??nnes wrote:

Den 06.03.2018 09.56, skrev Daniel Vetter:

On Thu, Feb 22, 2018 at 09:06:50PM +0100, Noralf Tr??nnes wrote:

This adds an API for writing in-kernel clients.

TODO:
- Flesh out and complete documentation.
- Cloned displays is not tested.
- Complete tiled display support and test it.
- Test plug/unplug different monitors.
- A runtime knob to prevent clients from attaching for debugging purposes.
- Maybe a way to unbind individual client instances.
- Maybe take the sysrq support in drm_fb_helper and move it here somehow.
- Add suspend/resume callbacks.
Does anyone know why fbdev requires suspend/resume?

Signed-off-by: Noralf Tr??nnes 

The core client api I like. Some of the opens I'm seeing:

- If we go with using the internal kms api directly instead of IOCTL
wrappers then a huge pile of the functions you have here aren't needed
(e.g. all the event stuff we can just directly use vblank events instead
of all the wrapping). I'm leaning ever more into that direction, since
much less code to add.

Looking at drm_fb_helper once again I now see an opportunity to simplify
the modesetting code by nuking drm_fb_helper_connector and stop
maintaining an array of connectors. It looks to be possible to just
create an array temporarily in drm_setup_crtcs() for the duration of the
function. The connectors we care about are ref counted and attached to
modesets. This would remove the need for drm_fb_helper_add_one_connector().

So I might be able to do struct drm_fb_helper_crtc -> drm_client_crtc
and let the client API take over drm_setup_crtcs(). I'll give it a try.

I'm more wondering why we need drm_client_crtc at all, why is drm_crtc not
good enough. Or maybe I'm missing something. Imo ioctl wrappers should be
the exception where we really, really need them (because the backend of
the ioctl isn't implemented in a generic way, e.g. dumb buffers), not for
stuff where we already have a perfectly useable in-kernel abi (anything
related to modesetting).


I was talking about moving the modesetting code from drm_fb_helper.c
to drm_client.c, which meant moving 'struct drm_fb_helper_crtc' as well.

struct drm_fb_helper_crtc {
    struct drm_mode_set mode_set;
    struct drm_display_mode *desired_mode;
    int x, y;
};

But maybe we can get rid of that struct as well.
The info it contains is also available in drm_mode_set:

static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
                u32 width, u32 height)
{
...
    drm_fb_helper_for_each_connector(fb_helper, i) {
        struct drm_display_mode *mode = modes[i];
        struct drm_fb_helper_crtc *fb_crtc = crtcs[i];
        struct drm_fb_offset *offset = [i];

        if (mode && fb_crtc) {
            struct drm_mode_set *modeset = _crtc->mode_set;

            fb_crtc->desired_mode = mode;
            fb_crtc->x = offset->x;
            fb_crtc->y = offset->y;
            modeset->mode = drm_mode_duplicate(dev,
                               fb_crtc->desired_mode);
            modeset->x = offset->x;
            modeset->y = offset->y;
        }
    }

I took the hint about my ioctl wrappers :-)

Noralf.



And in a way the ioctl wrappers wouldn't really be ioctl wrappers
conceptually, but simple share some of the same code with the ioctl call
chain. The idea is to provide some minimal wrappar around the ->dumb*
callbacks.

Anything else is not needed, I think.


There is one challenge I see upfront and that's the i915 fb_helper
callback in drm_setup_crtcs().


- The register/unregister model needs more thought. Allowing both clients
to register whenever they want to, and drm_device instances to come and
go is what fbcon has done, and the resulting locking is a horror show.

I think if we require that all in-kernel drm_clients are registers when
loading drm.ko (and enabled/disabled only per module options and
Kconfig), then we can throw out all the locking. That avoids a lot of
the headaches.

2nd, if the list of clients is static over the lifetime of drm.ko, we
also don't need to iterate existing drivers. Which avoids me having to
review the iterator patch (that's the other aspect where fbcon totally
falls over and essentially just ignores a bunch of races).

Are you talking about linking the clients into drm.ko?

drivers/gpu/drm/Makefile:

drm-$(CONFIG_DRM_CLIENT_BOOTSPLASH) += client/drm_bootsplash.o

drivers/gpu/drm/drm_drv.c:

??static int __init drm_core_init(void)
??{
+?? drm_bootsplash_register();
+?? drm_fbdev_register();
??}

drivers/gpu/drm/drm_internal.h:

#ifdef DRM_CLIENT_BOOTSPLASH
void drm_bootsplash_register(void);
#else
static inline void drm_bootsplash_register(void)
{
}
#endif

drivers/gpu/drm/client/drm_bootsplash.c:

static const struct drm_client_funcs drm_bootsplash_funcs = {
?? .name?? ?? = "drm_bootsplash",
?? 

Re: [RFC v3 09/12] drm: Add API for in-kernel clients

[Daniel Vetter]

On Thu, Mar 08, 2018 at 06:12:11PM +0100, Noralf Tr??nnes wrote:
> 
> Den 06.03.2018 09.56, skrev Daniel Vetter:
> > On Thu, Feb 22, 2018 at 09:06:50PM +0100, Noralf Tr??nnes wrote:
> > > This adds an API for writing in-kernel clients.
> > > 
> > > TODO:
> > > - Flesh out and complete documentation.
> > > - Cloned displays is not tested.
> > > - Complete tiled display support and test it.
> > > - Test plug/unplug different monitors.
> > > - A runtime knob to prevent clients from attaching for debugging purposes.
> > > - Maybe a way to unbind individual client instances.
> > > - Maybe take the sysrq support in drm_fb_helper and move it here somehow.
> > > - Add suspend/resume callbacks.
> > >Does anyone know why fbdev requires suspend/resume?
> > > 
> > > Signed-off-by: Noralf Tr??nnes 
> > The core client api I like. Some of the opens I'm seeing:
> > 
> > - If we go with using the internal kms api directly instead of IOCTL
> >wrappers then a huge pile of the functions you have here aren't needed
> >(e.g. all the event stuff we can just directly use vblank events instead
> >of all the wrapping). I'm leaning ever more into that direction, since
> >much less code to add.
> 
> Looking at drm_fb_helper once again I now see an opportunity to simplify
> the modesetting code by nuking drm_fb_helper_connector and stop
> maintaining an array of connectors. It looks to be possible to just
> create an array temporarily in drm_setup_crtcs() for the duration of the
> function. The connectors we care about are ref counted and attached to
> modesets. This would remove the need for drm_fb_helper_add_one_connector().
> 
> So I might be able to do struct drm_fb_helper_crtc -> drm_client_crtc
> and let the client API take over drm_setup_crtcs(). I'll give it a try.

I'm more wondering why we need drm_client_crtc at all, why is drm_crtc not
good enough. Or maybe I'm missing something. Imo ioctl wrappers should be
the exception where we really, really need them (because the backend of
the ioctl isn't implemented in a generic way, e.g. dumb buffers), not for
stuff where we already have a perfectly useable in-kernel abi (anything
related to modesetting).

And in a way the ioctl wrappers wouldn't really be ioctl wrappers
conceptually, but simple share some of the same code with the ioctl call
chain. The idea is to provide some minimal wrappar around the ->dumb*
callbacks.

Anything else is not needed, I think.

> There is one challenge I see upfront and that's the i915 fb_helper
> callback in drm_setup_crtcs().
> 
> > - The register/unregister model needs more thought. Allowing both clients
> >to register whenever they want to, and drm_device instances to come and
> >go is what fbcon has done, and the resulting locking is a horror show.
> > 
> >I think if we require that all in-kernel drm_clients are registers when
> >loading drm.ko (and enabled/disabled only per module options and
> >Kconfig), then we can throw out all the locking. That avoids a lot of
> >the headaches.
> > 
> >2nd, if the list of clients is static over the lifetime of drm.ko, we
> >also don't need to iterate existing drivers. Which avoids me having to
> >review the iterator patch (that's the other aspect where fbcon totally
> >falls over and essentially just ignores a bunch of races).
> 
> Are you talking about linking the clients into drm.ko?
> 
> drivers/gpu/drm/Makefile:
> 
> drm-$(CONFIG_DRM_CLIENT_BOOTSPLASH) += client/drm_bootsplash.o
> 
> drivers/gpu/drm/drm_drv.c:
> 
> ??static int __init drm_core_init(void)
> ??{
> +?? drm_bootsplash_register();
> +?? drm_fbdev_register();
> ??}
> 
> drivers/gpu/drm/drm_internal.h:
> 
> #ifdef DRM_CLIENT_BOOTSPLASH
> void drm_bootsplash_register(void);
> #else
> static inline void drm_bootsplash_register(void)
> {
> }
> #endif
> 
> drivers/gpu/drm/client/drm_bootsplash.c:
> 
> static const struct drm_client_funcs drm_bootsplash_funcs = {
> ?? .name?? ?? = "drm_bootsplash",
> ?? .new?? ?? = drm_bootsplash_new,
> ?? .remove?? ?? = drm_bootsplash_remove,
> ?? .hotplug?? = drm_bootsplash_hotplug,
> };
> 
> void drm_bootsplash_register(void)
> {
> ?? drm_client_register(_bootsplash_funcs);
> }
> 
> drivers/gpu/drm/drm_client.c:
> 
> static LIST_HEAD(drm_client_funcs_list);
> 
> void drm_client_register(const struct drm_client_funcs *funcs)
> {
> ?? struct drm_client_funcs_entry *funcs_entry;
> 
> ?? funcs_entry = kzalloc(sizeof(*funcs_entry), GFP_KERNEL);
> ?? if (!funcs_entry) {
> ?? ?? DRM_ERROR("Failed to register: %s\n", funcs->name);
> ?? ?? return;
> ?? }
> 
> ?? funcs_entry->funcs = funcs;
> 
> ?? list_add(_entry->list, _client_funcs_list);
> 
> ?? DRM_DEBUG_KMS("%s\n", funcs->name);
> }
> 
> 
> And each client having a runtime enable/disable knob:
> 
> drivers/gpu/drm/client/drm_bootsplash.c:
> 
> static bool drm_bootsplash_enabled = true;

Re: [RFC v3 09/12] drm: Add API for in-kernel clients

[Noralf Trønnes]

Den 06.03.2018 09.56, skrev Daniel Vetter:

On Thu, Feb 22, 2018 at 09:06:50PM +0100, Noralf Trønnes wrote:

This adds an API for writing in-kernel clients.

TODO:
- Flesh out and complete documentation.
- Cloned displays is not tested.
- Complete tiled display support and test it.
- Test plug/unplug different monitors.
- A runtime knob to prevent clients from attaching for debugging purposes.
- Maybe a way to unbind individual client instances.
- Maybe take the sysrq support in drm_fb_helper and move it here somehow.
- Add suspend/resume callbacks.
   Does anyone know why fbdev requires suspend/resume?

Signed-off-by: Noralf Trønnes 

The core client api I like. Some of the opens I'm seeing:

- If we go with using the internal kms api directly instead of IOCTL
   wrappers then a huge pile of the functions you have here aren't needed
   (e.g. all the event stuff we can just directly use vblank events instead
   of all the wrapping). I'm leaning ever more into that direction, since
   much less code to add.


Looking at drm_fb_helper once again I now see an opportunity to simplify
the modesetting code by nuking drm_fb_helper_connector and stop
maintaining an array of connectors. It looks to be possible to just
create an array temporarily in drm_setup_crtcs() for the duration of the
function. The connectors we care about are ref counted and attached to
modesets. This would remove the need for drm_fb_helper_add_one_connector().

So I might be able to do struct drm_fb_helper_crtc -> drm_client_crtc
and let the client API take over drm_setup_crtcs(). I'll give it a try.

There is one challenge I see upfront and that's the i915 fb_helper
callback in drm_setup_crtcs().


- The register/unregister model needs more thought. Allowing both clients
   to register whenever they want to, and drm_device instances to come and
   go is what fbcon has done, and the resulting locking is a horror show.

   I think if we require that all in-kernel drm_clients are registers when
   loading drm.ko (and enabled/disabled only per module options and
   Kconfig), then we can throw out all the locking. That avoids a lot of
   the headaches.

   2nd, if the list of clients is static over the lifetime of drm.ko, we
   also don't need to iterate existing drivers. Which avoids me having to
   review the iterator patch (that's the other aspect where fbcon totally
   falls over and essentially just ignores a bunch of races).


Are you talking about linking the clients into drm.ko?

drivers/gpu/drm/Makefile:

drm-$(CONFIG_DRM_CLIENT_BOOTSPLASH) += client/drm_bootsplash.o

drivers/gpu/drm/drm_drv.c:

 static int __init drm_core_init(void)
 {
+    drm_bootsplash_register();
+    drm_fbdev_register();
 }

drivers/gpu/drm/drm_internal.h:

#ifdef DRM_CLIENT_BOOTSPLASH
void drm_bootsplash_register(void);
#else
static inline void drm_bootsplash_register(void)
{
}
#endif

drivers/gpu/drm/client/drm_bootsplash.c:

static const struct drm_client_funcs drm_bootsplash_funcs = {
    .name        = "drm_bootsplash",
    .new        = drm_bootsplash_new,
    .remove        = drm_bootsplash_remove,
    .hotplug    = drm_bootsplash_hotplug,
};

void drm_bootsplash_register(void)
{
    drm_client_register(_bootsplash_funcs);
}

drivers/gpu/drm/drm_client.c:

static LIST_HEAD(drm_client_funcs_list);

void drm_client_register(const struct drm_client_funcs *funcs)
{
    struct drm_client_funcs_entry *funcs_entry;

    funcs_entry = kzalloc(sizeof(*funcs_entry), GFP_KERNEL);
    if (!funcs_entry) {
        DRM_ERROR("Failed to register: %s\n", funcs->name);
        return;
    }

    funcs_entry->funcs = funcs;

    list_add(_entry->list, _client_funcs_list);

    DRM_DEBUG_KMS("%s\n", funcs->name);
}


And each client having a runtime enable/disable knob:

drivers/gpu/drm/client/drm_bootsplash.c:

static bool drm_bootsplash_enabled = true;
module_param_named(bootsplash_enabled, drm_bootsplash_enabled, bool, 0600);
MODULE_PARM_DESC(bootsplash_enabled, "Enable bootsplash client 
[default=true]");



Simple USB Display
A few months back while looking at the udl shmem code, I got the idea
that I could turn a Raspberry Pi Zero into a $5 USB to HDMI/DSI/DPI/DBI/TV
adapter. The host side would be a simple tinydrm driver using the kernel
compression lib to speed up transfers. The gadget/device side would be a
userspace app decompressing the buffer into an exported dumb buffer.

While working with this client API I realized that I could use it and
write a kernel gadget driver instead avoiding the challenge of going
back and forth to userspace with the framebuffer. For such a client I
would have preferred it to be a loadable module not linked into drm.ko
to increase the chance that distributions would enable it.

Noralf.


---
  drivers/gpu/drm/Kconfig |2 +
  drivers/gpu/drm/Makefile|3 +-
  drivers/gpu/drm/client/Kconfig  |4 +
  drivers/gpu/drm/client/Makefile |1 +
  

Re: [RFC v3 09/12] drm: Add API for in-kernel clients

[Daniel Vetter]

On Thu, Feb 22, 2018 at 09:06:50PM +0100, Noralf Trønnes wrote:
> This adds an API for writing in-kernel clients.
> 
> TODO:
> - Flesh out and complete documentation.
> - Cloned displays is not tested.
> - Complete tiled display support and test it.
> - Test plug/unplug different monitors.
> - A runtime knob to prevent clients from attaching for debugging purposes.
> - Maybe a way to unbind individual client instances.
> - Maybe take the sysrq support in drm_fb_helper and move it here somehow.
> - Add suspend/resume callbacks.
>   Does anyone know why fbdev requires suspend/resume?
> 
> Signed-off-by: Noralf Trønnes 

The core client api I like. Some of the opens I'm seeing:

- If we go with using the internal kms api directly instead of IOCTL
  wrappers then a huge pile of the functions you have here aren't needed
  (e.g. all the event stuff we can just directly use vblank events instead
  of all the wrapping). I'm leaning ever more into that direction, since
  much less code to add.

- The register/unregister model needs more thought. Allowing both clients
  to register whenever they want to, and drm_device instances to come and
  go is what fbcon has done, and the resulting locking is a horror show.

  I think if we require that all in-kernel drm_clients are registers when
  loading drm.ko (and enabled/disabled only per module options and
  Kconfig), then we can throw out all the locking. That avoids a lot of
  the headaches.

  2nd, if the list of clients is static over the lifetime of drm.ko, we
  also don't need to iterate existing drivers. Which avoids me having to
  review the iterator patch (that's the other aspect where fbcon totally
  falls over and essentially just ignores a bunch of races).

> ---
>  drivers/gpu/drm/Kconfig |2 +
>  drivers/gpu/drm/Makefile|3 +-
>  drivers/gpu/drm/client/Kconfig  |4 +
>  drivers/gpu/drm/client/Makefile |1 +
>  drivers/gpu/drm/client/drm_client.c | 1612 
> +++

I'd move this into main drm/ directory, it's fairly core stuff.

>  drivers/gpu/drm/drm_drv.c   |6 +
>  drivers/gpu/drm/drm_file.c  |3 +
>  drivers/gpu/drm/drm_probe_helper.c  |3 +
>  include/drm/drm_client.h|  192 +
>  include/drm/drm_device.h|1 +
>  include/drm/drm_file.h  |7 +
>  11 files changed, 1833 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/client/Kconfig
>  create mode 100644 drivers/gpu/drm/client/Makefile
>  create mode 100644 drivers/gpu/drm/client/drm_client.c
>  create mode 100644 include/drm/drm_client.h
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index deeefa7a1773..d4ae15f9ee9f 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -154,6 +154,8 @@ config DRM_SCHED
>   tristate
>   depends on DRM
>  
> +source "drivers/gpu/drm/client/Kconfig"
> +
>  source "drivers/gpu/drm/i2c/Kconfig"
>  
>  source "drivers/gpu/drm/arm/Kconfig"
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 50093ff4479b..8e06dc7eeca1 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -18,7 +18,7 @@ drm-y   :=  drm_auth.o drm_bufs.o drm_cache.o \
>   drm_encoder.o drm_mode_object.o drm_property.o \
>   drm_plane.o drm_color_mgmt.o drm_print.o \
>   drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
> - drm_syncobj.o drm_lease.o
> + drm_syncobj.o drm_lease.o client/drm_client.o
>  
>  drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
>  drm-$(CONFIG_DRM_VM) += drm_vm.o
> @@ -103,3 +103,4 @@ obj-$(CONFIG_DRM_MXSFB)   += mxsfb/
>  obj-$(CONFIG_DRM_TINYDRM) += tinydrm/
>  obj-$(CONFIG_DRM_PL111) += pl111/
>  obj-$(CONFIG_DRM_TVE200) += tve200/
> +obj-y+= client/
> diff --git a/drivers/gpu/drm/client/Kconfig b/drivers/gpu/drm/client/Kconfig
> new file mode 100644
> index ..4bb8e4655ff7
> --- /dev/null
> +++ b/drivers/gpu/drm/client/Kconfig
> @@ -0,0 +1,4 @@
> +menu "DRM Clients"
> + depends on DRM
> +
> +endmenu
> diff --git a/drivers/gpu/drm/client/Makefile b/drivers/gpu/drm/client/Makefile
> new file mode 100644
> index ..f66554cd5c45
> --- /dev/null
> +++ b/drivers/gpu/drm/client/Makefile
> @@ -0,0 +1 @@
> +# SPDX-License-Identifier: GPL-2.0
> diff --git a/drivers/gpu/drm/client/drm_client.c 
> b/drivers/gpu/drm/client/drm_client.c
> new file mode 100644
> index ..a633bf747316
> --- /dev/null
> +++ b/drivers/gpu/drm/client/drm_client.c
> @@ -0,0 +1,1612 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright 2018 Noralf Trønnes
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "drm_crtc_internal.h"
> +#include "drm_internal.h"
> +
> +struct drm_client_funcs_entry {
> + 

[RFC v3 09/12] drm: Add API for in-kernel clients

[Noralf Trønnes]

This adds an API for writing in-kernel clients.

TODO:
- Flesh out and complete documentation.
- Cloned displays is not tested.
- Complete tiled display support and test it.
- Test plug/unplug different monitors.
- A runtime knob to prevent clients from attaching for debugging purposes.
- Maybe a way to unbind individual client instances.
- Maybe take the sysrq support in drm_fb_helper and move it here somehow.
- Add suspend/resume callbacks.
  Does anyone know why fbdev requires suspend/resume?

Signed-off-by: Noralf Trønnes 
---
 drivers/gpu/drm/Kconfig |2 +
 drivers/gpu/drm/Makefile|3 +-
 drivers/gpu/drm/client/Kconfig  |4 +
 drivers/gpu/drm/client/Makefile |1 +
 drivers/gpu/drm/client/drm_client.c | 1612 +++
 drivers/gpu/drm/drm_drv.c   |6 +
 drivers/gpu/drm/drm_file.c  |3 +
 drivers/gpu/drm/drm_probe_helper.c  |3 +
 include/drm/drm_client.h|  192 +
 include/drm/drm_device.h|1 +
 include/drm/drm_file.h  |7 +
 11 files changed, 1833 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/client/Kconfig
 create mode 100644 drivers/gpu/drm/client/Makefile
 create mode 100644 drivers/gpu/drm/client/drm_client.c
 create mode 100644 include/drm/drm_client.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index deeefa7a1773..d4ae15f9ee9f 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -154,6 +154,8 @@ config DRM_SCHED
tristate
depends on DRM
 
+source "drivers/gpu/drm/client/Kconfig"
+
 source "drivers/gpu/drm/i2c/Kconfig"
 
 source "drivers/gpu/drm/arm/Kconfig"
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 50093ff4479b..8e06dc7eeca1 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -18,7 +18,7 @@ drm-y   :=drm_auth.o drm_bufs.o drm_cache.o \
drm_encoder.o drm_mode_object.o drm_property.o \
drm_plane.o drm_color_mgmt.o drm_print.o \
drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
-   drm_syncobj.o drm_lease.o
+   drm_syncobj.o drm_lease.o client/drm_client.o
 
 drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
 drm-$(CONFIG_DRM_VM) += drm_vm.o
@@ -103,3 +103,4 @@ obj-$(CONFIG_DRM_MXSFB) += mxsfb/
 obj-$(CONFIG_DRM_TINYDRM) += tinydrm/
 obj-$(CONFIG_DRM_PL111) += pl111/
 obj-$(CONFIG_DRM_TVE200) += tve200/
+obj-y  += client/
diff --git a/drivers/gpu/drm/client/Kconfig b/drivers/gpu/drm/client/Kconfig
new file mode 100644
index ..4bb8e4655ff7
--- /dev/null
+++ b/drivers/gpu/drm/client/Kconfig
@@ -0,0 +1,4 @@
+menu "DRM Clients"
+   depends on DRM
+
+endmenu
diff --git a/drivers/gpu/drm/client/Makefile b/drivers/gpu/drm/client/Makefile
new file mode 100644
index ..f66554cd5c45
--- /dev/null
+++ b/drivers/gpu/drm/client/Makefile
@@ -0,0 +1 @@
+# SPDX-License-Identifier: GPL-2.0
diff --git a/drivers/gpu/drm/client/drm_client.c 
b/drivers/gpu/drm/client/drm_client.c
new file mode 100644
index ..a633bf747316
--- /dev/null
+++ b/drivers/gpu/drm/client/drm_client.c
@@ -0,0 +1,1612 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright 2018 Noralf Trønnes
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "drm_crtc_internal.h"
+#include "drm_internal.h"
+
+struct drm_client_funcs_entry {
+   struct list_head list;
+   const struct drm_client_funcs *funcs;
+};
+
+static LIST_HEAD(drm_client_list);
+static LIST_HEAD(drm_client_funcs_list);
+static DEFINE_MUTEX(drm_client_list_lock);
+
+static void drm_client_new(struct drm_device *dev,
+  const struct drm_client_funcs *funcs)
+{
+   struct drm_client_dev *client;
+   int ret;
+
+   lockdep_assert_held(_client_list_lock);
+
+   client = kzalloc(sizeof(*client), GFP_KERNEL);
+   if (!client)
+   return;
+
+   mutex_init(>lock);
+   client->dev = dev;
+   client->funcs = funcs;
+
+   ret = funcs->new(client);
+   DRM_DEV_DEBUG_KMS(dev->dev, "%s: ret=%d\n", funcs->name, ret);
+   if (ret) {
+   drm_client_free(client);
+   return;
+   }
+
+   list_add(>list, _client_list);
+}
+
+/**
+ * drm_client_free - Free DRM client resources
+ * @client: DRM client
+ *
+ * This is called automatically on client removal unless the client returns
+ * non-zero in the _client_funcs->remove callback. The fbdev client does
+ * this when it can't close _file because userspace has an open fd.
+ */
+void drm_client_free(struct drm_client_dev *client)
+{
+   DRM_DEV_DEBUG_KMS(client->dev->dev, "%s\n", client->funcs->name);
+   if (WARN_ON(client->file)) {
+   client->file_ref_count = 1;
+   drm_client_put_file(client);
+   }
+