diff mbox

[3/4] drm: Add helper for simple display pipeline

Message ID 1462454674-2246-4-git-send-email-noralf@tronnes.org (mailing list archive)
State New, archived
Headers show

Commit Message

Noralf Trønnes May 5, 2016, 1:24 p.m. UTC
Provides helper functions for drivers that have a simple display
pipeline. Plane, crtc and encoder are collapsed into one entity.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/Kconfig                 |   7 ++
 drivers/gpu/drm/Makefile                |   1 +
 drivers/gpu/drm/drm_simple_kms_helper.c | 157 ++++++++++++++++++++++++++++++++
 include/drm/drm_simple_kms_helper.h     |  88 ++++++++++++++++++
 4 files changed, 253 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_simple_kms_helper.c
 create mode 100644 include/drm/drm_simple_kms_helper.h

Comments

Daniel Vetter May 5, 2016, 4:45 p.m. UTC | #1
On Thu, May 05, 2016 at 03:24:33PM +0200, Noralf Trønnes wrote:
> Provides helper functions for drivers that have a simple display
> pipeline. Plane, crtc and encoder are collapsed into one entity.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

When resubmitting a patch please always have a per-patch changelog of what
you've done. Otherwise you force reviewers to remember they looked at this
already, dig out the old mail and compare ;-)

> ---
>  drivers/gpu/drm/Kconfig                 |   7 ++
>  drivers/gpu/drm/Makefile                |   1 +
>  drivers/gpu/drm/drm_simple_kms_helper.c | 157 ++++++++++++++++++++++++++++++++
>  include/drm/drm_simple_kms_helper.h     |  88 ++++++++++++++++++

Please also add a new section (next to all the ones for the other kms
helpers) in Documentation/DocBook/gpu.tmpl and pull your kerneldoc in.
Please also add a short overview kernel doc section to the .c file (Using
the DOC: header) and also pull that into the overall gpu docs.

You can check the results using

$ make htmldocs

>  4 files changed, 253 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_simple_kms_helper.c
>  create mode 100644 include/drm/drm_simple_kms_helper.h
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 16e4c21..ff9f480 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -39,6 +39,13 @@ config DRM_KMS_HELPER
>  	help
>  	  CRTC helpers for KMS drivers.
>  
> +config DRM_SIMPLE_KMS_HELPER
> +	tristate
> +	depends on DRM
> +	select DRM_KMS_HELPER
> +	help
> +	  Helpers for very simple KMS drivers.
> +
>  config DRM_KMS_FB_HELPER
>  	bool
>  	depends on DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 43c2abf..7e99923 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -31,6 +31,7 @@ drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
>  drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
>  
>  obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
> +obj-$(CONFIG_DRM_SIMPLE_KMS_HELPER) += drm_simple_kms_helper.o
>  
>  CFLAGS_drm_trace_points.o := -I$(src)
>  
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> new file mode 100644
> index 0000000..c8725bb
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -0,0 +1,157 @@
> +/*
> + * 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.
> + */
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_plane_helper.h>
> +#include <drm/drm_simple_kms_helper.h>
> +#include <linux/slab.h>
> +
> +static const struct drm_encoder_funcs drm_simple_kms_encoder_funcs = {
> +	.destroy = drm_encoder_cleanup,
> +};
> +
> +static void drm_simple_kms_crtc_enable(struct drm_crtc *crtc)
> +{
> +	struct drm_simple_display_pipe *pipe;
> +
> +	pipe = container_of(crtc, struct drm_simple_display_pipe, crtc);
> +	if (!pipe->funcs || !pipe->funcs->enable)
> +		return;
> +
> +	pipe->funcs->enable(pipe, crtc->state);
> +}
> +
> +static void drm_simple_kms_crtc_disable(struct drm_crtc *crtc)
> +{
> +	struct drm_simple_display_pipe *pipe;
> +
> +	pipe = container_of(crtc, struct drm_simple_display_pipe, crtc);
> +	if (!pipe->funcs || !pipe->funcs->disable)
> +		return;
> +
> +	pipe->funcs->disable(pipe);
> +}
> +
> +static const struct drm_crtc_helper_funcs drm_simple_kms_crtc_helper_funcs = {
> +	.disable = drm_simple_kms_crtc_disable,
> +	.enable = drm_simple_kms_crtc_enable,
> +};
> +
> +static const struct drm_crtc_funcs drm_simple_kms_crtc_funcs = {
> +	.reset = drm_atomic_helper_crtc_reset,
> +	.destroy = drm_crtc_cleanup,
> +	.set_config = drm_atomic_helper_set_config,
> +	.page_flip = drm_atomic_helper_page_flip,
> +	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
> +};
> +
> +static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
> +					struct drm_plane_state *pstate)
> +{
> +	struct drm_simple_display_pipe *pipe;
> +	struct drm_crtc_state *cstate;
> +
> +	pipe = container_of(plane, struct drm_simple_display_pipe, plane);
> +	if (!pipe->funcs || !pipe->funcs->check)
> +		return 0;
> +
> +	cstate = drm_atomic_get_existing_crtc_state(pstate->state,
> +						    &pipe->crtc);
> +
> +	return pipe->funcs->check(pipe, pstate, cstate);
> +}
> +
> +static void drm_simple_kms_plane_atomic_update(struct drm_plane *plane,
> +					struct drm_plane_state *pstate)
> +{
> +	struct drm_simple_display_pipe *pipe;
> +
> +	pipe = container_of(plane, struct drm_simple_display_pipe, plane);
> +	if (!pipe->funcs || !pipe->funcs->update)
> +		return;
> +
> +	pipe->funcs->update(pipe, pstate);
> +}
> +
> +static const struct drm_plane_helper_funcs drm_simple_kms_plane_helper_funcs = {
> +	.atomic_check = drm_simple_kms_plane_atomic_check,
> +	.atomic_update = drm_simple_kms_plane_atomic_update,
> +};
> +
> +static const struct drm_plane_funcs drm_simple_kms_plane_funcs = {
> +	.update_plane		= drm_atomic_helper_update_plane,
> +	.disable_plane		= drm_atomic_helper_disable_plane,
> +	.destroy		= drm_plane_cleanup,
> +	.reset			= drm_atomic_helper_plane_reset,
> +	.atomic_duplicate_state	= drm_atomic_helper_plane_duplicate_state,
> +	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
> +};
> +
> +/**
> + * drm_simple_display_pipe_init - Initialize a simple display pipeline
> + * @dev: DRM device
> + * @pipe: simple display pipe object to initialize
> + * @funcs: callbacks for the display pipe
> + * @formats: array of supported formats (%DRM_FORMAT_*)
> + * @format_count: number of elements in @formats
> + * @connector: connector to attach and register
> + *
> + * Sets up a display pipeline which consist of a really simple
> + * plane-crtc-encoder pipe coupled with the provided connector.

need to specify that @funcs is optional.

> + *
> + * Returns:
> + * Zero on success, error code on failure.

"_negative_ error code"
> + */
> +int drm_simple_display_pipe_init(struct drm_device *dev,
> +				 struct drm_simple_display_pipe *pipe,
> +				 struct drm_simple_display_pipe_funcs *funcs,
> +				 const uint32_t *formats,
> +				 unsigned int format_count,
> +				 struct drm_connector *connector)
> +{
> +	struct drm_encoder *encoder = &pipe->encoder;
> +	struct drm_plane *plane = &pipe->plane;
> +	struct drm_crtc *crtc = &pipe->crtc;
> +	int ret;
> +
> +	pipe->funcs = funcs;
> +
> +	drm_plane_helper_add(plane, &drm_simple_kms_plane_helper_funcs);
> +	ret = drm_universal_plane_init(dev, plane, 0,
> +				       &drm_simple_kms_plane_funcs,
> +				       formats, format_count,
> +				       DRM_PLANE_TYPE_PRIMARY, NULL);
> +	if (ret)
> +		return ret;
> +
> +	drm_crtc_helper_add(crtc, &drm_simple_kms_crtc_helper_funcs);
> +	ret = drm_crtc_init_with_planes(dev, crtc, plane, NULL,
> +					&drm_simple_kms_crtc_funcs, NULL);
> +	if (ret)
> +		return ret;
> +
> +	encoder->possible_crtcs = 1 << drm_crtc_index(crtc);
> +	ret = drm_encoder_init(dev, encoder, &drm_simple_kms_encoder_funcs,
> +			       DRM_MODE_ENCODER_NONE, NULL);
> +	if (ret)
> +		return ret;
> +
> +	ret = drm_mode_connector_attach_encoder(connector, encoder);
> +	if (ret)
> +		return ret;
> +
> +	return drm_connector_register(connector);
> +}
> +EXPORT_SYMBOL(drm_simple_display_pipe_init);
> +
> +MODULE_LICENSE("GPL");
> diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
> new file mode 100644
> index 0000000..69eac94
> --- /dev/null
> +++ b/include/drm/drm_simple_kms_helper.h
> @@ -0,0 +1,88 @@
> +/*
> + * 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.
> + */
> +
> +#ifndef __LINUX_DRM_SIMPLE_KMS_HELPER_H
> +#define __LINUX_DRM_SIMPLE_KMS_HELPER_H
> +
> +struct drm_simple_display_pipe;
> +
> +/**
> + * struct drm_simple_display_pipe_funcs - helper operations for a simple
> + *                                        display pipeline
> + */
> +struct drm_simple_display_pipe_funcs {
> +	/**
> +	 * @enable:
> +	 *
> +	 * This function should be used to enable the pipeline.
> +	 * It is called when the underlying crtc is enabled.

Please add boilerplate to all these hooks along the lines of:

"This hook is optional."

Besides that little bit of polish looks all good to me.
-Daniel

> +	 */
> +	void (*enable)(struct drm_simple_display_pipe *pipe,
> +		       struct drm_crtc_state *crtc_state);
> +	/**
> +	 * @disable:
> +	 *
> +	 * This function should be used to disable the pipeline.
> +	 * It is called when the underlying crtc is disabled.
> +	 */
> +	void (*disable)(struct drm_simple_display_pipe *pipe);
> +
> +	/**
> +	 * @check:
> +	 *
> +	 * This function is called in the check phase of an atomic update,
> +	 * specifically when the underlying plane is checked.
> +	 *
> +	 * RETURNS:
> +	 *
> +	 * 0 on success, -EINVAL if the state or the transition can't be
> +	 * supported, -ENOMEM on memory allocation failure and -EDEADLK if an
> +	 * attempt to obtain another state object ran into a &drm_modeset_lock
> +	 * deadlock.
> +	 */
> +	int (*check)(struct drm_simple_display_pipe *pipe,
> +		     struct drm_plane_state *plane_state,
> +		     struct drm_crtc_state *crtc_state);
> +	/**
> +	 * @update:
> +	 *
> +	 * This function is called when the underlying plane state is updated.
> +	 */
> +	void (*update)(struct drm_simple_display_pipe *pipe,
> +		       struct drm_plane_state *plane_state);
> +};
> +
> +/**
> + * struct drm_simple_display_pipe - simple display pipeline
> + * @crtc: CRTC control structure
> + * @plane: Plane control structure
> + * @encoder: Encoder control structure
> + * @connector: Connector control structure
> + * @funcs: Pipeline control functions (optional)
> + *
> + * Simple display pipeline with plane, crtc and encoder collapsed into one
> + * entity.
> + */
> +struct drm_simple_display_pipe {
> +	struct drm_crtc crtc;
> +	struct drm_plane plane;
> +	struct drm_encoder encoder;
> +	struct drm_connector *connector;
> +
> +	struct drm_simple_display_pipe_funcs *funcs;
> +};
> +
> +int drm_simple_display_pipe_init(struct drm_device *dev,
> +				 struct drm_simple_display_pipe *pipe,
> +				 struct drm_simple_display_pipe_funcs *funcs,
> +				 const uint32_t *formats,
> +				 unsigned int format_count,
> +				 struct drm_connector *connector);
> +
> +#endif /* __LINUX_DRM_SIMPLE_KMS_HELPER_H */
> -- 
> 2.2.2
>
Daniel Vetter May 9, 2016, 2:46 p.m. UTC | #2
On Thu, May 05, 2016 at 03:24:33PM +0200, Noralf Trønnes wrote:
> Provides helper functions for drivers that have a simple display
> pipeline. Plane, crtc and encoder are collapsed into one entity.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

> +static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
> +					struct drm_plane_state *pstate)
> +{
> +	struct drm_simple_display_pipe *pipe;
> +	struct drm_crtc_state *cstate;
> +
> +	pipe = container_of(plane, struct drm_simple_display_pipe, plane);
> +	if (!pipe->funcs || !pipe->funcs->check)
> +		return 0;
> +
> +	cstate = drm_atomic_get_existing_crtc_state(pstate->state,
> +						    &pipe->crtc);
> +
> +	return pipe->funcs->check(pipe, pstate, cstate);
> +}

Ok one thing I've missed here is that for most drivers this is way too
simple a check function, which means we'll end up with tons of duplicated
code. Things which the drm core allows, but simple pipelines all don't
really cope with:
- plane scaling
- disabling the plane without the crtc (i.e. scan out black)
- plane not sized to fill the entire hactive/vactive

There's a helper to do most of these checks for you -
drm_plane_helper_check_update. I think it'd be good to place a call for
that in here, before we call down into the driver's ->check callback. But
ofc before we return 0; we want these checks always done. And catch all
these things so that drivers never fall over this pitfall.

Noticed while discussing tilcdc atomic patches, since tilcdc could
probably use drm_simple_display_pipe too.
-Daniel
Noralf Trønnes May 9, 2016, 6:37 p.m. UTC | #3
Den 09.05.2016 16:46, skrev Daniel Vetter:
> On Thu, May 05, 2016 at 03:24:33PM +0200, Noralf Trønnes wrote:
>> Provides helper functions for drivers that have a simple display
>> pipeline. Plane, crtc and encoder are collapsed into one entity.
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> +static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
>> +					struct drm_plane_state *pstate)
>> +{
>> +	struct drm_simple_display_pipe *pipe;
>> +	struct drm_crtc_state *cstate;
>> +
>> +	pipe = container_of(plane, struct drm_simple_display_pipe, plane);
>> +	if (!pipe->funcs || !pipe->funcs->check)
>> +		return 0;
>> +
>> +	cstate = drm_atomic_get_existing_crtc_state(pstate->state,
>> +						    &pipe->crtc);
>> +
>> +	return pipe->funcs->check(pipe, pstate, cstate);
>> +}
> Ok one thing I've missed here is that for most drivers this is way too
> simple a check function, which means we'll end up with tons of duplicated
> code. Things which the drm core allows, but simple pipelines all don't
> really cope with:
> - plane scaling
> - disabling the plane without the crtc (i.e. scan out black)
> - plane not sized to fill the entire hactive/vactive
>
> There's a helper to do most of these checks for you -
> drm_plane_helper_check_update. I think it'd be good to place a call for
> that in here, before we call down into the driver's ->check callback. But
> ofc before we return 0; we want these checks always done. And catch all
> these things so that drivers never fall over this pitfall.

Does this resemble what you're after? I'm just guessing here.

static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
                     struct drm_plane_state *pstate)
{
     struct drm_rect src = {
         .x1 = pstate->src_x,
         .y1 = pstate->src_y,
         .x2 = pstate->src_x + pstate->src_w,
         .y2 = pstate->src_y + pstate->src_h,
     };
     struct drm_rect dest = {
         .x1 = pstate->crtc_x,
         .y1 = pstate->crtc_y,
         .x2 = pstate->crtc_x + pstate->crtc_w,
         .y2 = pstate->crtc_y + pstate->crtc_h,
     };
     struct drm_rect clip = { 0 };
     struct drm_simple_display_pipe *pipe;
     struct drm_crtc_state *cstate;
     bool visible;
     int ret;

     pipe = container_of(plane, struct drm_simple_display_pipe, plane);
     clip.x2 = pipe->crtc.mode.hdisplay;
     clip.y2 = pipe->crtc.mode.vdisplay;
     ret = drm_plane_helper_check_update(plane, &pipe->crtc, plane->fb,
                         &src, &dest, &clip,
                         DRM_PLANE_HELPER_NO_SCALING,
                         DRM_PLANE_HELPER_NO_SCALING,
                         false, false, &visible);
     if (ret)
         return ret;

     /* How to handle !visible, is it even possible? */

     if (!pipe->funcs || !pipe->funcs->check)
         return 0;

     cstate = drm_atomic_get_existing_crtc_state(pstate->state,
                             &pipe->crtc);

     return pipe->funcs->check(pipe, pstate, cstate);
}


Noralf.

> Noticed while discussing tilcdc atomic patches, since tilcdc could
> probably use drm_simple_display_pipe too.
> -Daniel
Daniel Vetter May 10, 2016, 6:59 a.m. UTC | #4
On Mon, May 09, 2016 at 08:37:39PM +0200, Noralf Trønnes wrote:
> 
> Den 09.05.2016 16:46, skrev Daniel Vetter:
> >On Thu, May 05, 2016 at 03:24:33PM +0200, Noralf Trønnes wrote:
> >>Provides helper functions for drivers that have a simple display
> >>pipeline. Plane, crtc and encoder are collapsed into one entity.
> >>
> >>Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> >>+static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
> >>+					struct drm_plane_state *pstate)
> >>+{
> >>+	struct drm_simple_display_pipe *pipe;
> >>+	struct drm_crtc_state *cstate;
> >>+
> >>+	pipe = container_of(plane, struct drm_simple_display_pipe, plane);
> >>+	if (!pipe->funcs || !pipe->funcs->check)
> >>+		return 0;
> >>+
> >>+	cstate = drm_atomic_get_existing_crtc_state(pstate->state,
> >>+						    &pipe->crtc);
> >>+
> >>+	return pipe->funcs->check(pipe, pstate, cstate);
> >>+}
> >Ok one thing I've missed here is that for most drivers this is way too
> >simple a check function, which means we'll end up with tons of duplicated
> >code. Things which the drm core allows, but simple pipelines all don't
> >really cope with:
> >- plane scaling
> >- disabling the plane without the crtc (i.e. scan out black)
> >- plane not sized to fill the entire hactive/vactive
> >
> >There's a helper to do most of these checks for you -
> >drm_plane_helper_check_update. I think it'd be good to place a call for
> >that in here, before we call down into the driver's ->check callback. But
> >ofc before we return 0; we want these checks always done. And catch all
> >these things so that drivers never fall over this pitfall.
> 
> Does this resemble what you're after? I'm just guessing here.
> 
> static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
>                     struct drm_plane_state *pstate)
> {
>     struct drm_rect src = {
>         .x1 = pstate->src_x,
>         .y1 = pstate->src_y,
>         .x2 = pstate->src_x + pstate->src_w,
>         .y2 = pstate->src_y + pstate->src_h,
>     };
>     struct drm_rect dest = {
>         .x1 = pstate->crtc_x,
>         .y1 = pstate->crtc_y,
>         .x2 = pstate->crtc_x + pstate->crtc_w,
>         .y2 = pstate->crtc_y + pstate->crtc_h,
>     };
>     struct drm_rect clip = { 0 };

Clip rect needs to be set to crtc_state->adjusted_mode.h/vdisplay, see
rockchip or armada. Otherwise you'll clip to nothing and always fail.

>     struct drm_simple_display_pipe *pipe;
>     struct drm_crtc_state *cstate;
>     bool visible;
>     int ret;
> 
>     pipe = container_of(plane, struct drm_simple_display_pipe, plane);
>     clip.x2 = pipe->crtc.mode.hdisplay;
>     clip.y2 = pipe->crtc.mode.vdisplay;
>     ret = drm_plane_helper_check_update(plane, &pipe->crtc, plane->fb,
>                         &src, &dest, &clip,
>                         DRM_PLANE_HELPER_NO_SCALING,
>                         DRM_PLANE_HELPER_NO_SCALING,
>                         false, false, &visible);

can_update_disabled = true should work, Only caveat is that you might get
a call to update the primary plane when the display is turned off.
Probably best though if we handle that in the simple pipe driver too. Just
call drm_atomic_helper_commit_planes with active_only = true.

>     if (ret)
>         return ret;
> 
>     /* How to handle !visible, is it even possible? */

	if (!visible)
		return -EINVAL;

You can't, so need to reject it.
> 
>     if (!pipe->funcs || !pipe->funcs->check)
>         return 0;
> 
>     cstate = drm_atomic_get_existing_crtc_state(pstate->state,
>                             &pipe->crtc);
> 
>     return pipe->funcs->check(pipe, pstate, cstate);
> }

Cheers, Daniel
Daniel Vetter May 10, 2016, 10:36 p.m. UTC | #5
On Tue, May 10, 2016 at 8:59 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>>     if (ret)
>>         return ret;
>>
>>     /* How to handle !visible, is it even possible? */
>
>         if (!visible)
>                 return -EINVAL;
>
> You can't, so need to reject it.

Ok, on further thought I think we need a bit more here. We not just
need to make sure the plane is visible and not scaled or positioned,
but also that the plane is only enabled iff the crtc is.

So even before you call anything else you need to have this check:

if (crtc_state->enable != !!plane_state->crtc)
        return -EINVAL; /* plane must match crtc enable state */

if (!crtc_state->enable)
        return 0; /* nothing to check when disabling or disabled
already, calling check helpers and driver callbacks might only result
in confusion in such a case. */

Then continue with the remaining check logic that you have already,
with my coments in the earlier mail addressed.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 16e4c21..ff9f480 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -39,6 +39,13 @@  config DRM_KMS_HELPER
 	help
 	  CRTC helpers for KMS drivers.
 
+config DRM_SIMPLE_KMS_HELPER
+	tristate
+	depends on DRM
+	select DRM_KMS_HELPER
+	help
+	  Helpers for very simple KMS drivers.
+
 config DRM_KMS_FB_HELPER
 	bool
 	depends on DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 43c2abf..7e99923 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -31,6 +31,7 @@  drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
 drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
 
 obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
+obj-$(CONFIG_DRM_SIMPLE_KMS_HELPER) += drm_simple_kms_helper.o
 
 CFLAGS_drm_trace_points.o := -I$(src)
 
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
new file mode 100644
index 0000000..c8725bb
--- /dev/null
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -0,0 +1,157 @@ 
+/*
+ * 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.
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_plane_helper.h>
+#include <drm/drm_simple_kms_helper.h>
+#include <linux/slab.h>
+
+static const struct drm_encoder_funcs drm_simple_kms_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
+static void drm_simple_kms_crtc_enable(struct drm_crtc *crtc)
+{
+	struct drm_simple_display_pipe *pipe;
+
+	pipe = container_of(crtc, struct drm_simple_display_pipe, crtc);
+	if (!pipe->funcs || !pipe->funcs->enable)
+		return;
+
+	pipe->funcs->enable(pipe, crtc->state);
+}
+
+static void drm_simple_kms_crtc_disable(struct drm_crtc *crtc)
+{
+	struct drm_simple_display_pipe *pipe;
+
+	pipe = container_of(crtc, struct drm_simple_display_pipe, crtc);
+	if (!pipe->funcs || !pipe->funcs->disable)
+		return;
+
+	pipe->funcs->disable(pipe);
+}
+
+static const struct drm_crtc_helper_funcs drm_simple_kms_crtc_helper_funcs = {
+	.disable = drm_simple_kms_crtc_disable,
+	.enable = drm_simple_kms_crtc_enable,
+};
+
+static const struct drm_crtc_funcs drm_simple_kms_crtc_funcs = {
+	.reset = drm_atomic_helper_crtc_reset,
+	.destroy = drm_crtc_cleanup,
+	.set_config = drm_atomic_helper_set_config,
+	.page_flip = drm_atomic_helper_page_flip,
+	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
+};
+
+static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
+					struct drm_plane_state *pstate)
+{
+	struct drm_simple_display_pipe *pipe;
+	struct drm_crtc_state *cstate;
+
+	pipe = container_of(plane, struct drm_simple_display_pipe, plane);
+	if (!pipe->funcs || !pipe->funcs->check)
+		return 0;
+
+	cstate = drm_atomic_get_existing_crtc_state(pstate->state,
+						    &pipe->crtc);
+
+	return pipe->funcs->check(pipe, pstate, cstate);
+}
+
+static void drm_simple_kms_plane_atomic_update(struct drm_plane *plane,
+					struct drm_plane_state *pstate)
+{
+	struct drm_simple_display_pipe *pipe;
+
+	pipe = container_of(plane, struct drm_simple_display_pipe, plane);
+	if (!pipe->funcs || !pipe->funcs->update)
+		return;
+
+	pipe->funcs->update(pipe, pstate);
+}
+
+static const struct drm_plane_helper_funcs drm_simple_kms_plane_helper_funcs = {
+	.atomic_check = drm_simple_kms_plane_atomic_check,
+	.atomic_update = drm_simple_kms_plane_atomic_update,
+};
+
+static const struct drm_plane_funcs drm_simple_kms_plane_funcs = {
+	.update_plane		= drm_atomic_helper_update_plane,
+	.disable_plane		= drm_atomic_helper_disable_plane,
+	.destroy		= drm_plane_cleanup,
+	.reset			= drm_atomic_helper_plane_reset,
+	.atomic_duplicate_state	= drm_atomic_helper_plane_duplicate_state,
+	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
+};
+
+/**
+ * drm_simple_display_pipe_init - Initialize a simple display pipeline
+ * @dev: DRM device
+ * @pipe: simple display pipe object to initialize
+ * @funcs: callbacks for the display pipe
+ * @formats: array of supported formats (%DRM_FORMAT_*)
+ * @format_count: number of elements in @formats
+ * @connector: connector to attach and register
+ *
+ * Sets up a display pipeline which consist of a really simple
+ * plane-crtc-encoder pipe coupled with the provided connector.
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drm_simple_display_pipe_init(struct drm_device *dev,
+				 struct drm_simple_display_pipe *pipe,
+				 struct drm_simple_display_pipe_funcs *funcs,
+				 const uint32_t *formats,
+				 unsigned int format_count,
+				 struct drm_connector *connector)
+{
+	struct drm_encoder *encoder = &pipe->encoder;
+	struct drm_plane *plane = &pipe->plane;
+	struct drm_crtc *crtc = &pipe->crtc;
+	int ret;
+
+	pipe->funcs = funcs;
+
+	drm_plane_helper_add(plane, &drm_simple_kms_plane_helper_funcs);
+	ret = drm_universal_plane_init(dev, plane, 0,
+				       &drm_simple_kms_plane_funcs,
+				       formats, format_count,
+				       DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (ret)
+		return ret;
+
+	drm_crtc_helper_add(crtc, &drm_simple_kms_crtc_helper_funcs);
+	ret = drm_crtc_init_with_planes(dev, crtc, plane, NULL,
+					&drm_simple_kms_crtc_funcs, NULL);
+	if (ret)
+		return ret;
+
+	encoder->possible_crtcs = 1 << drm_crtc_index(crtc);
+	ret = drm_encoder_init(dev, encoder, &drm_simple_kms_encoder_funcs,
+			       DRM_MODE_ENCODER_NONE, NULL);
+	if (ret)
+		return ret;
+
+	ret = drm_mode_connector_attach_encoder(connector, encoder);
+	if (ret)
+		return ret;
+
+	return drm_connector_register(connector);
+}
+EXPORT_SYMBOL(drm_simple_display_pipe_init);
+
+MODULE_LICENSE("GPL");
diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
new file mode 100644
index 0000000..69eac94
--- /dev/null
+++ b/include/drm/drm_simple_kms_helper.h
@@ -0,0 +1,88 @@ 
+/*
+ * 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.
+ */
+
+#ifndef __LINUX_DRM_SIMPLE_KMS_HELPER_H
+#define __LINUX_DRM_SIMPLE_KMS_HELPER_H
+
+struct drm_simple_display_pipe;
+
+/**
+ * struct drm_simple_display_pipe_funcs - helper operations for a simple
+ *                                        display pipeline
+ */
+struct drm_simple_display_pipe_funcs {
+	/**
+	 * @enable:
+	 *
+	 * This function should be used to enable the pipeline.
+	 * It is called when the underlying crtc is enabled.
+	 */
+	void (*enable)(struct drm_simple_display_pipe *pipe,
+		       struct drm_crtc_state *crtc_state);
+	/**
+	 * @disable:
+	 *
+	 * This function should be used to disable the pipeline.
+	 * It is called when the underlying crtc is disabled.
+	 */
+	void (*disable)(struct drm_simple_display_pipe *pipe);
+
+	/**
+	 * @check:
+	 *
+	 * This function is called in the check phase of an atomic update,
+	 * specifically when the underlying plane is checked.
+	 *
+	 * RETURNS:
+	 *
+	 * 0 on success, -EINVAL if the state or the transition can't be
+	 * supported, -ENOMEM on memory allocation failure and -EDEADLK if an
+	 * attempt to obtain another state object ran into a &drm_modeset_lock
+	 * deadlock.
+	 */
+	int (*check)(struct drm_simple_display_pipe *pipe,
+		     struct drm_plane_state *plane_state,
+		     struct drm_crtc_state *crtc_state);
+	/**
+	 * @update:
+	 *
+	 * This function is called when the underlying plane state is updated.
+	 */
+	void (*update)(struct drm_simple_display_pipe *pipe,
+		       struct drm_plane_state *plane_state);
+};
+
+/**
+ * struct drm_simple_display_pipe - simple display pipeline
+ * @crtc: CRTC control structure
+ * @plane: Plane control structure
+ * @encoder: Encoder control structure
+ * @connector: Connector control structure
+ * @funcs: Pipeline control functions (optional)
+ *
+ * Simple display pipeline with plane, crtc and encoder collapsed into one
+ * entity.
+ */
+struct drm_simple_display_pipe {
+	struct drm_crtc crtc;
+	struct drm_plane plane;
+	struct drm_encoder encoder;
+	struct drm_connector *connector;
+
+	struct drm_simple_display_pipe_funcs *funcs;
+};
+
+int drm_simple_display_pipe_init(struct drm_device *dev,
+				 struct drm_simple_display_pipe *pipe,
+				 struct drm_simple_display_pipe_funcs *funcs,
+				 const uint32_t *formats,
+				 unsigned int format_count,
+				 struct drm_connector *connector);
+
+#endif /* __LINUX_DRM_SIMPLE_KMS_HELPER_H */