| Message ID | 1460135110-24121-4-git-send-email-noralf@tronnes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Apr 08, 2016 at 07:05:05PM +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> Looks good. A few comments below, but the big thing is adding kerneldoc and cross-referencing it (e.g. from drm_panel to drm_panel_connector). -Daniel > --- > drivers/gpu/drm/Kconfig | 7 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/drm_simple_kms_helper.c | 262 ++++++++++++++++++++++++++++++++ > include/drm/drm_simple_kms_helper.h | 44 ++++++ > 4 files changed, 314 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 8ae7ab6..cb62cd9 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -31,6 +31,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 61766de..ea9bf59 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -28,6 +28,7 @@ drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o > drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.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..e193f7db > --- /dev/null > +++ b/drivers/gpu/drm/drm_simple_kms_helper.c > @@ -0,0 +1,262 @@ > +/* > + * 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_helper.h> > +#include <drm/drm_crtc_helper.h> > +#include <drm/drm_panel.h> > +#include <drm/drm_plane_helper.h> > +#include <drm/drm_simple_kms_helper.h> > +#include <linux/slab.h> > + > +struct drm_simple_kms_connector { > + struct drm_connector base; > + struct drm_panel *panel; > +}; > + > +static inline struct drm_simple_kms_connector * > +to_simple_connector(struct drm_connector *connector) > +{ > + return container_of(connector, struct drm_simple_kms_connector, base); > +} > + > +static int drm_simple_kms_connector_get_modes(struct drm_connector *connector) > +{ > + return drm_panel_get_modes(to_simple_connector(connector)->panel); > +} > + > +static struct drm_encoder * > +drm_simple_kms_connector_best_encoder(struct drm_connector *connector) > +{ > + return drm_encoder_find(connector->dev, connector->encoder_ids[0]); > +} I think this would be useful for atomic drivers in general. I even thought we have it already somewhere ... > + > +static const struct drm_connector_helper_funcs drm_simple_kms_connector_helper_funcs = { > + .get_modes = drm_simple_kms_connector_get_modes, > + .best_encoder = drm_simple_kms_connector_best_encoder, > +}; > + > +static enum drm_connector_status > +drm_simple_kms_connector_detect(struct drm_connector *connector, bool force) > +{ > + if (drm_device_is_unplugged(connector->dev)) > + return connector_status_disconnected; > + > + return connector->status; > +} > + > +static void drm_simple_kms_connector_destroy(struct drm_connector *connector) > +{ > + struct drm_simple_kms_connector *panel_connector; > + > + panel_connector = to_simple_connector(connector); > + drm_panel_detach(panel_connector->panel); > + drm_panel_remove(panel_connector->panel); > + drm_connector_unregister(connector); > + drm_connector_cleanup(connector); > + kfree(panel_connector); > +} > + > +static const struct drm_connector_funcs drm_simple_kms_connector_funcs = { > + .dpms = drm_atomic_helper_connector_dpms, > + .reset = drm_atomic_helper_connector_reset, > + .detect = drm_simple_kms_connector_detect, > + .fill_modes = drm_helper_probe_single_connector_modes, > + .destroy = drm_simple_kms_connector_destroy, > + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > +}; drm_simple_kms_panel_connector is toooooooooooo long ;-) What about just drm_panel_connector instead? Also not entirely sure whether this is best placed here, or better in drm_panel.c, or maybe even in a new drm_panel_helper.c. Imo drm_panel_connector and drm_simple_display_pipe should work as orthogonal helpers. Or 3rd option: You just expose the above helpers for detect/get_modes only. > + > +/** > + * drm_simple_kms_panel_connector_create - Create simple connector for panel > + * @dev: DRM device > + * @panel: DRM panel > + * @connector_type: user visible type of the connector > + * > + * Creates a simple connector for a panel. > + * The panel needs to provide a get_modes() function. > + * > + * Returns: > + * Pointer to new connector or ERR_PTR on failure. > + */ > +struct drm_connector * > +drm_simple_kms_panel_connector_create(struct drm_device *dev, > + struct drm_panel *panel, > + int connector_type) > +{ > + struct drm_simple_kms_connector *panel_connector; > + struct drm_connector *connector; > + int ret; > + > + panel_connector = kzalloc(sizeof(*panel_connector), GFP_KERNEL); > + if (!panel_connector) > + return ERR_PTR(-ENOMEM); > + > + panel_connector->panel = panel; > + connector = &panel_connector->base; > + drm_connector_helper_add(connector, &drm_simple_kms_connector_helper_funcs); > + ret = drm_connector_init(dev, connector, &drm_simple_kms_connector_funcs, > + connector_type); > + if (ret) { > + kfree(panel_connector); > + return ERR_PTR(ret); > + } > + > + connector->status = connector_status_connected; > + drm_panel_init(panel); > + drm_panel_add(panel); > + drm_panel_attach(panel, connector); > + > + return connector; > +} > +EXPORT_SYMBOL(drm_simple_kms_panel_connector_create); > + > +static void drm_simple_kms_encoder_disable(struct drm_encoder *encoder) > +{ > +} > + > +static void drm_simple_kms_encoder_enable(struct drm_encoder *encoder) > +{ > +} > + > +static int drm_simple_kms_encoder_atomic_check(struct drm_encoder *encoder, > + struct drm_crtc_state *crtc_state, > + struct drm_connector_state *conn_state) > +{ > + return 0; > +} Above dummy functions shouldn't be necessary. > + > +static const struct drm_encoder_helper_funcs drm_simple_kms_encoder_helper_funcs = { > + .disable = drm_simple_kms_encoder_disable, > + .enable = drm_simple_kms_encoder_enable, > + .atomic_check = drm_simple_kms_encoder_atomic_check, > +}; > + > +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 void drm_simple_kms_plane_atomic_update(struct drm_plane *plane, > + struct drm_plane_state *old_state) > +{ > + struct drm_simple_display_pipe *pipe; > + > + pipe = container_of(plane, struct drm_simple_display_pipe, plane); > + if (!pipe->funcs || !pipe->funcs->plane_update) > + return; > + > + pipe->funcs->plane_update(pipe, old_state); > +} > + > +static const struct drm_plane_helper_funcs drm_simple_kms_plane_helper_funcs = { > + .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 pipe > + * @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 pipe 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); > + drm_encoder_helper_add(encoder, &drm_simple_kms_encoder_helper_funcs); > + 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..0f7461b > --- /dev/null > +++ b/include/drm/drm_simple_kms_helper.h > @@ -0,0 +1,44 @@ > +/* > + * 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 { > + void (*enable)(struct drm_simple_display_pipe *pipe, > + struct drm_crtc_state *crtc_state); > + void (*disable)(struct drm_simple_display_pipe *pipe); > + void (*plane_update)(struct drm_simple_display_pipe *pipe, > + struct drm_plane_state *plane_state); On 2nd thought maybe we want an atomic_check in here too, with same parameters *pipe, *crtc_state and *plane_state (so that it's useful for both plane-only updates and modeset changes). > +}; > + > +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); > +struct drm_connector * > +drm_simple_kms_panel_connector_create(struct drm_device *dev, > + struct drm_panel *panel, > + int connector_type); > + > + > + > +#endif /* __LINUX_DRM_SIMPLE_KMS_HELPER_H */ > -- > 2.2.2 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Den 13.04.2016 13:05, skrev Daniel Vetter: > On Fri, Apr 08, 2016 at 07:05:05PM +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 void drm_simple_kms_encoder_disable(struct drm_encoder *encoder) >> +{ >> +} >> + >> +static void drm_simple_kms_encoder_enable(struct drm_encoder *encoder) >> +{ >> +} >> + >> +static int drm_simple_kms_encoder_atomic_check(struct drm_encoder *encoder, >> + struct drm_crtc_state *crtc_state, >> + struct drm_connector_state *conn_state) >> +{ >> + return 0; >> +} > Above dummy functions shouldn't be necessary. I could remove .atomic_check, but not .disable and .enable which gave me NULL pointer errors. I need .enable or .commit, and .disable or .dpms. Gist: void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev, struct drm_atomic_state *old_state) { for_each_connector_in_state(old_state, connector, old_conn_state, i) { const struct drm_encoder_helper_funcs *funcs; funcs = encoder->helper_private; if (funcs->enable) funcs->enable(encoder); else funcs->commit(encoder); static void disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) { for_each_connector_in_state(old_state, connector, old_conn_state, i) { const struct drm_encoder_helper_funcs *funcs; funcs = encoder->helper_private; if (connector->state->crtc && funcs->prepare) funcs->prepare(encoder); else if (funcs->disable) funcs->disable(encoder); else funcs->dpms(encoder, DRM_MODE_DPMS_OFF); > >> + >> +static const struct drm_encoder_helper_funcs drm_simple_kms_encoder_helper_funcs = { >> + .disable = drm_simple_kms_encoder_disable, >> + .enable = drm_simple_kms_encoder_enable, >> + .atomic_check = drm_simple_kms_encoder_atomic_check, >> +}; >> + >> +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 void drm_simple_kms_plane_atomic_update(struct drm_plane *plane, >> + struct drm_plane_state *old_state) >> +{ >> + struct drm_simple_display_pipe *pipe; >> + >> + pipe = container_of(plane, struct drm_simple_display_pipe, plane); >> + if (!pipe->funcs || !pipe->funcs->plane_update) >> + return; >> + >> + pipe->funcs->plane_update(pipe, old_state); >> +} >> + >> +static const struct drm_plane_helper_funcs drm_simple_kms_plane_helper_funcs = { >> + .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 pipe >> + * @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 pipe 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); >> + drm_encoder_helper_add(encoder, &drm_simple_kms_encoder_helper_funcs); >> + 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..0f7461b >> --- /dev/null >> +++ b/include/drm/drm_simple_kms_helper.h >> @@ -0,0 +1,44 @@ >> +/* >> + * 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 { >> + void (*enable)(struct drm_simple_display_pipe *pipe, >> + struct drm_crtc_state *crtc_state); >> + void (*disable)(struct drm_simple_display_pipe *pipe); >> + void (*plane_update)(struct drm_simple_display_pipe *pipe, >> + struct drm_plane_state *plane_state); > On 2nd thought maybe we want an atomic_check in here too, with same > parameters *pipe, *crtc_state and *plane_state (so that it's useful for > both plane-only updates and modeset changes). I could do this for the plane: struct drm_simple_display_pipe_funcs { void (*plane_check)(struct drm_simple_display_pipe *pipe, struct drm_plane_state *plane_state); } static void drm_simple_kms_plane_atomic_check(struct drm_plane *plane, struct drm_plane_state *state) { struct drm_simple_display_pipe *pipe; pipe = container_of(plane, struct drm_simple_display_pipe, plane); if (!pipe->funcs || !pipe->funcs->plane_check) return; pipe->funcs->plane_check(pipe, state); } static const struct drm_plane_helper_funcs drm_simple_kms_plane_helper_funcs = { .atomic_check = drm_simple_kms_plane_atomic_check, }; But how should I do crtc_state ? Noralf. >> +}; >> + >> +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); >> +struct drm_connector * >> +drm_simple_kms_panel_connector_create(struct drm_device *dev, >> + struct drm_panel *panel, >> + int connector_type); >> + >> + >> + >> +#endif /* __LINUX_DRM_SIMPLE_KMS_HELPER_H */ >> -- >> 2.2.2 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, May 02, 2016 at 05:55:15PM +0200, Noralf Trønnes wrote: > > Den 13.04.2016 13:05, skrev Daniel Vetter: > >On Fri, Apr 08, 2016 at 07:05:05PM +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 void drm_simple_kms_encoder_disable(struct drm_encoder *encoder) > >>+{ > >>+} > >>+ > >>+static void drm_simple_kms_encoder_enable(struct drm_encoder *encoder) > >>+{ > >>+} > >>+ > >>+static int drm_simple_kms_encoder_atomic_check(struct drm_encoder *encoder, > >>+ struct drm_crtc_state *crtc_state, > >>+ struct drm_connector_state *conn_state) > >>+{ > >>+ return 0; > >>+} > >Above dummy functions shouldn't be necessary. > > I could remove .atomic_check, but not .disable and .enable which gave me > NULL pointer errors. I need .enable or .commit, and .disable or .dpms. > > Gist: > > void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev, > struct drm_atomic_state > *old_state) > { > for_each_connector_in_state(old_state, connector, old_conn_state, i) > { > const struct drm_encoder_helper_funcs *funcs; > > funcs = encoder->helper_private; > > if (funcs->enable) > funcs->enable(encoder); > else > funcs->commit(encoder); > > static void > disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) > { > for_each_connector_in_state(old_state, connector, old_conn_state, i) > { > const struct drm_encoder_helper_funcs *funcs; > > funcs = encoder->helper_private; > > if (connector->state->crtc && funcs->prepare) > funcs->prepare(encoder); > else if (funcs->disable) > funcs->disable(encoder); > else > funcs->dpms(encoder, DRM_MODE_DPMS_OFF); Hm right. I'm voting that we make them all optional, there's been an effort all over the place to not force drivers to type in silly dummy funcs. > >>+ > >>+static const struct drm_encoder_helper_funcs drm_simple_kms_encoder_helper_funcs = { > >>+ .disable = drm_simple_kms_encoder_disable, > >>+ .enable = drm_simple_kms_encoder_enable, > >>+ .atomic_check = drm_simple_kms_encoder_atomic_check, > >>+}; > >>+ > >>+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 void drm_simple_kms_plane_atomic_update(struct drm_plane *plane, > >>+ struct drm_plane_state *old_state) > >>+{ > >>+ struct drm_simple_display_pipe *pipe; > >>+ > >>+ pipe = container_of(plane, struct drm_simple_display_pipe, plane); > >>+ if (!pipe->funcs || !pipe->funcs->plane_update) > >>+ return; > >>+ > >>+ pipe->funcs->plane_update(pipe, old_state); > >>+} > >>+ > >>+static const struct drm_plane_helper_funcs drm_simple_kms_plane_helper_funcs = { > >>+ .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 pipe > >>+ * @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 pipe 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); > >>+ drm_encoder_helper_add(encoder, &drm_simple_kms_encoder_helper_funcs); > >>+ 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..0f7461b > >>--- /dev/null > >>+++ b/include/drm/drm_simple_kms_helper.h > >>@@ -0,0 +1,44 @@ > >>+/* > >>+ * 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 { > >>+ void (*enable)(struct drm_simple_display_pipe *pipe, > >>+ struct drm_crtc_state *crtc_state); > >>+ void (*disable)(struct drm_simple_display_pipe *pipe); > >>+ void (*plane_update)(struct drm_simple_display_pipe *pipe, > >>+ struct drm_plane_state *plane_state); > >On 2nd thought maybe we want an atomic_check in here too, with same > >parameters *pipe, *crtc_state and *plane_state (so that it's useful for > >both plane-only updates and modeset changes). > > I could do this for the plane: > > struct drm_simple_display_pipe_funcs { > void (*plane_check)(struct drm_simple_display_pipe *pipe, > struct drm_plane_state *plane_state); > } > > static void drm_simple_kms_plane_atomic_check(struct drm_plane *plane, > struct drm_plane_state *state) > { > struct drm_simple_display_pipe *pipe; > > pipe = container_of(plane, struct drm_simple_display_pipe, plane); > if (!pipe->funcs || !pipe->funcs->plane_check) > return; > > pipe->funcs->plane_check(pipe, state); > } > > static const struct drm_plane_helper_funcs drm_simple_kms_plane_helper_funcs > = { > .atomic_check = drm_simple_kms_plane_atomic_check, > }; > > But how should I do crtc_state ? Atomic core guarantees that if you have a plane state, then the crtc state for that plane is always hanging around. You should be able to get at it (but only for atomic_check functions, there's a different way in the commit hooks) using: drm_atomic_get_existing_plane_state(plane_state->state, plane_state->crtc); Yes I know I should get around to documeting all the structures we have ... Unfortunately it's a bit a mess and I'd like to clean up the code organization first a bit. Cheers, Daniel
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 8ae7ab6..cb62cd9 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -31,6 +31,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 61766de..ea9bf59 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -28,6 +28,7 @@ drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.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..e193f7db --- /dev/null +++ b/drivers/gpu/drm/drm_simple_kms_helper.c @@ -0,0 +1,262 @@ +/* + * 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_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_panel.h> +#include <drm/drm_plane_helper.h> +#include <drm/drm_simple_kms_helper.h> +#include <linux/slab.h> + +struct drm_simple_kms_connector { + struct drm_connector base; + struct drm_panel *panel; +}; + +static inline struct drm_simple_kms_connector * +to_simple_connector(struct drm_connector *connector) +{ + return container_of(connector, struct drm_simple_kms_connector, base); +} + +static int drm_simple_kms_connector_get_modes(struct drm_connector *connector) +{ + return drm_panel_get_modes(to_simple_connector(connector)->panel); +} + +static struct drm_encoder * +drm_simple_kms_connector_best_encoder(struct drm_connector *connector) +{ + return drm_encoder_find(connector->dev, connector->encoder_ids[0]); +} + +static const struct drm_connector_helper_funcs drm_simple_kms_connector_helper_funcs = { + .get_modes = drm_simple_kms_connector_get_modes, + .best_encoder = drm_simple_kms_connector_best_encoder, +}; + +static enum drm_connector_status +drm_simple_kms_connector_detect(struct drm_connector *connector, bool force) +{ + if (drm_device_is_unplugged(connector->dev)) + return connector_status_disconnected; + + return connector->status; +} + +static void drm_simple_kms_connector_destroy(struct drm_connector *connector) +{ + struct drm_simple_kms_connector *panel_connector; + + panel_connector = to_simple_connector(connector); + drm_panel_detach(panel_connector->panel); + drm_panel_remove(panel_connector->panel); + drm_connector_unregister(connector); + drm_connector_cleanup(connector); + kfree(panel_connector); +} + +static const struct drm_connector_funcs drm_simple_kms_connector_funcs = { + .dpms = drm_atomic_helper_connector_dpms, + .reset = drm_atomic_helper_connector_reset, + .detect = drm_simple_kms_connector_detect, + .fill_modes = drm_helper_probe_single_connector_modes, + .destroy = drm_simple_kms_connector_destroy, + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, +}; + +/** + * drm_simple_kms_panel_connector_create - Create simple connector for panel + * @dev: DRM device + * @panel: DRM panel + * @connector_type: user visible type of the connector + * + * Creates a simple connector for a panel. + * The panel needs to provide a get_modes() function. + * + * Returns: + * Pointer to new connector or ERR_PTR on failure. + */ +struct drm_connector * +drm_simple_kms_panel_connector_create(struct drm_device *dev, + struct drm_panel *panel, + int connector_type) +{ + struct drm_simple_kms_connector *panel_connector; + struct drm_connector *connector; + int ret; + + panel_connector = kzalloc(sizeof(*panel_connector), GFP_KERNEL); + if (!panel_connector) + return ERR_PTR(-ENOMEM); + + panel_connector->panel = panel; + connector = &panel_connector->base; + drm_connector_helper_add(connector, &drm_simple_kms_connector_helper_funcs); + ret = drm_connector_init(dev, connector, &drm_simple_kms_connector_funcs, + connector_type); + if (ret) { + kfree(panel_connector); + return ERR_PTR(ret); + } + + connector->status = connector_status_connected; + drm_panel_init(panel); + drm_panel_add(panel); + drm_panel_attach(panel, connector); + + return connector; +} +EXPORT_SYMBOL(drm_simple_kms_panel_connector_create); + +static void drm_simple_kms_encoder_disable(struct drm_encoder *encoder) +{ +} + +static void drm_simple_kms_encoder_enable(struct drm_encoder *encoder) +{ +} + +static int drm_simple_kms_encoder_atomic_check(struct drm_encoder *encoder, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state) +{ + return 0; +} + +static const struct drm_encoder_helper_funcs drm_simple_kms_encoder_helper_funcs = { + .disable = drm_simple_kms_encoder_disable, + .enable = drm_simple_kms_encoder_enable, + .atomic_check = drm_simple_kms_encoder_atomic_check, +}; + +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 void drm_simple_kms_plane_atomic_update(struct drm_plane *plane, + struct drm_plane_state *old_state) +{ + struct drm_simple_display_pipe *pipe; + + pipe = container_of(plane, struct drm_simple_display_pipe, plane); + if (!pipe->funcs || !pipe->funcs->plane_update) + return; + + pipe->funcs->plane_update(pipe, old_state); +} + +static const struct drm_plane_helper_funcs drm_simple_kms_plane_helper_funcs = { + .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 pipe + * @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 pipe 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); + drm_encoder_helper_add(encoder, &drm_simple_kms_encoder_helper_funcs); + 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..0f7461b --- /dev/null +++ b/include/drm/drm_simple_kms_helper.h @@ -0,0 +1,44 @@ +/* + * 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 { + void (*enable)(struct drm_simple_display_pipe *pipe, + struct drm_crtc_state *crtc_state); + void (*disable)(struct drm_simple_display_pipe *pipe); + void (*plane_update)(struct drm_simple_display_pipe *pipe, + struct drm_plane_state *plane_state); +}; + +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); +struct drm_connector * +drm_simple_kms_panel_connector_create(struct drm_device *dev, + struct drm_panel *panel, + int connector_type); + + + +#endif /* __LINUX_DRM_SIMPLE_KMS_HELPER_H */
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 | 262 ++++++++++++++++++++++++++++++++ include/drm/drm_simple_kms_helper.h | 44 ++++++ 4 files changed, 314 insertions(+) create mode 100644 drivers/gpu/drm/drm_simple_kms_helper.c create mode 100644 include/drm/drm_simple_kms_helper.h