From 536e1715226c94037df12f7c6280cbe0f6009f92 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Wed, 5 Nov 2014 11:43:26 +0100 Subject: gpu: host1x: Call ->remove() only when a device is bound When a driver's ->probe() function fails, the host1x bus must not call its ->remove() function because the driver will already have cleaned up in the error handling path in ->probe(). Signed-off-by: Thierry Reding --- include/linux/host1x.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'include') diff --git a/include/linux/host1x.h b/include/linux/host1x.h index bb9840fd1e18..7890b553d12e 100644 --- a/include/linux/host1x.h +++ b/include/linux/host1x.h @@ -272,6 +272,8 @@ struct host1x_device { struct mutex clients_lock; struct list_head clients; + + bool bound; }; static inline struct host1x_device *to_host1x_device(struct device *dev) -- cgit v1.2.1 From f4c5cf88fbd50e4779042268947b2e2f90c20484 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Thu, 18 Dec 2014 15:29:14 +0100 Subject: gpu: host1x: Provide a proper struct bus_type Previously the struct bus_type exported by the host1x infrastructure was only a very basic skeleton. Turn that implementation into a more full- fledged bus to support proper probe ordering and power management. Note that the bus infrastructure needs to be available before any of the drivers can be registered. This is automatically ensured if all drivers are built as loadable modules (via symbol dependencies). If all drivers are built-in there are no such guarantees and the link order determines the initcall ordering. Adjust drivers/gpu/Makefile to make sure that the host1x bus infrastructure is initialized prior to any of its users (only drm/tegra currently). v2: Fix building host1x and tegra-drm as modules Reported-by: Dave Airlie Reviewed-by: Sean Paul Reviewed-by: Mark Zhang Signed-off-by: Thierry Reding --- include/linux/host1x.h | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) (limited to 'include') diff --git a/include/linux/host1x.h b/include/linux/host1x.h index 7890b553d12e..464f33814a94 100644 --- a/include/linux/host1x.h +++ b/include/linux/host1x.h @@ -250,17 +250,29 @@ void host1x_job_unpin(struct host1x_job *job); struct host1x_device; struct host1x_driver { + struct device_driver driver; + const struct of_device_id *subdevs; struct list_head list; - const char *name; int (*probe)(struct host1x_device *device); int (*remove)(struct host1x_device *device); + void (*shutdown)(struct host1x_device *device); }; -int host1x_driver_register(struct host1x_driver *driver); +static inline struct host1x_driver * +to_host1x_driver(struct device_driver *driver) +{ + return container_of(driver, struct host1x_driver, driver); +} + +int host1x_driver_register_full(struct host1x_driver *driver, + struct module *owner); void host1x_driver_unregister(struct host1x_driver *driver); +#define host1x_driver_register(driver) \ + host1x_driver_register_full(driver, THIS_MODULE) + struct host1x_device { struct host1x_driver *driver; struct list_head list; @@ -273,7 +285,7 @@ struct host1x_device { struct mutex clients_lock; struct list_head clients; - bool bound; + bool registered; }; static inline struct host1x_device *to_host1x_device(struct device *dev) -- cgit v1.2.1 From 3cad4b6887cd3d2b4e7f2c0ad6cb557b8eda4084 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Tue, 25 Nov 2014 13:05:12 +0100 Subject: drm/plane: Make ->atomic_update() mandatory There is no use-case where it would be useful for drivers not to implement this function and the transitional plane helpers already require drivers to provide an implementation. v2: add new requirement to kerneldoc Reviewed-by: Daniel Vetter Signed-off-by: Thierry Reding --- include/drm/drm_plane_helper.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include') diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h index a185392cafeb..06d0314a1352 100644 --- a/include/drm/drm_plane_helper.h +++ b/include/drm/drm_plane_helper.h @@ -52,7 +52,7 @@ extern int drm_crtc_init(struct drm_device *dev, * @prepare_fb: prepare a framebuffer for use by the plane * @cleanup_fb: cleanup a framebuffer when it's no longer used by the plane * @atomic_check: check that a given atomic state is valid and can be applied - * @atomic_update: apply an atomic state to the plane + * @atomic_update: apply an atomic state to the plane (mandatory) * * The helper operations are called by the mid-layer CRTC helper. */ -- cgit v1.2.1 From 407b8bd9f5d284ffa13a9f9a709e6289bb4ae47e Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Thu, 20 Nov 2014 12:05:50 +0100 Subject: drm/plane: Add optional ->atomic_disable() callback In order to prevent drivers from having to perform the same checks over and over again, add an optional ->atomic_disable callback which the core calls under the right circumstances. v2: pass old state and detect edges to avoid calling ->atomic_disable on already disabled planes, remove redundant comment (Daniel Vetter) v3: rename helper to drm_atomic_plane_disabling() to clarify that it is checking for transitions, move helper to drm_atomic_helper.h, clarify check for !old_state and its relation to transitional helpers Here's an extract from some discussion rationalizing the behaviour (for a full version, see the reference below): > > Hm, thinking about this some more this will result in a slight difference > > in behaviour, at least when drivers just use the helper ->reset functions > > but don't disable everything: > > - With transitional helpers we assume we know nothing and call > > ->atomic_disable. > > - With atomic old_state->crtc == NULL in the same situation right after > > boot-up, but we asssume the plane is really off and _dont_ call > > ->atomic_disable. > > > > Should we instead check for (old_state && old_state->crtc) and state that > > drivers need to make sure they don't have stuff hanging around? > > I don't think we can check for old_state because otherwise this will > always return false, whereas we really want it to force-disable planes > that could be on (lacking any more accurate information). For > transitional helpers anyway. > > For the atomic helpers, old_state will never be NULL, but I'd assume > that the driver would reconstruct the current state in ->reset(). By the way, the reason for why old_state can be NULL with transitional helpers is the ordering of the steps in the atomic transition. Currently the Tegra patches do this (based on your blog post and the Exynos proto- type): 1) atomic conversion, phase 1: - implement ->atomic_{check,update,disable}() - use drm_plane_helper_{update,disable}() 2) atomic conversion, phase 2: - call drm_mode_config_reset() from ->load() - implement ->reset() That's only a partial list of what's done in these steps, but that's the only relevant pieces for why old_state is NULL. What happens is that without ->reset() implemented there won't be any initial state, hence plane->state (the old_state here) will be NULL the first time atomic state is applied. We could of course reorder the sequence such that drivers are required to hook up ->reset() before they can (or at the same as they) hook up the transitional helpers. We could add an appropriate WARN_ON to this helper to make that more obvious. However, that will not solve the problem because it only gets rid of the special case. We still don't know whether old_state->crtc == NULL is the current state or just the initial default. So no matter which way we do this, I don't see a way to get away without requiring specific semantics from drivers. They would be that: - drivers recreate the correct state in ->reset() so that old_state->crtc != NULL if the plane is really enabled or - drivers have to ensure that the real state in fact mirrors the initial default as encoded in the state (plane disabled) References: http://lists.freedesktop.org/archives/dri-devel/2015-January/075578.html Reviewed-by: Daniel Vetter Reviewed-by: Gustavo Padovan Signed-off-by: Thierry Reding --- include/drm/drm_atomic_helper.h | 37 +++++++++++++++++++++++++++++++++++++ include/drm/drm_plane_helper.h | 3 +++ 2 files changed, 40 insertions(+) (limited to 'include') diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index 2095917ff8c7..a0ea4ded3cb5 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -127,4 +127,41 @@ void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector, #define drm_atomic_crtc_state_for_each_plane(plane, crtc_state) \ drm_for_each_plane_mask(plane, (crtc_state)->state->dev, (crtc_state)->plane_mask) +/* + * drm_atomic_plane_disabling - check whether a plane is being disabled + * @plane: plane object + * @old_state: previous atomic state + * + * Checks the atomic state of a plane to determine whether it's being disabled + * or not. This also WARNs if it detects an invalid state (both CRTC and FB + * need to either both be NULL or both be non-NULL). + * + * RETURNS: + * True if the plane is being disabled, false otherwise. + */ +static inline bool +drm_atomic_plane_disabling(struct drm_plane *plane, + struct drm_plane_state *old_state) +{ + /* + * When disabling a plane, CRTC and FB should always be NULL together. + * Anything else should be considered a bug in the atomic core, so we + * gently warn about it. + */ + WARN_ON((plane->state->crtc == NULL && plane->state->fb != NULL) || + (plane->state->crtc != NULL && plane->state->fb == NULL)); + + /* + * When using the transitional helpers, old_state may be NULL. If so, + * we know nothing about the current state and have to assume that it + * might be enabled. + * + * When using the atomic helpers, old_state won't be NULL. Therefore + * this check assumes that either the driver will have reconstructed + * the correct state in ->reset() or that the driver will have taken + * appropriate measures to disable all planes. + */ + return (!old_state || old_state->crtc) && !plane->state->crtc; +} + #endif /* DRM_ATOMIC_HELPER_H_ */ diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h index 06d0314a1352..31c11d36fae6 100644 --- a/include/drm/drm_plane_helper.h +++ b/include/drm/drm_plane_helper.h @@ -53,6 +53,7 @@ extern int drm_crtc_init(struct drm_device *dev, * @cleanup_fb: cleanup a framebuffer when it's no longer used by the plane * @atomic_check: check that a given atomic state is valid and can be applied * @atomic_update: apply an atomic state to the plane (mandatory) + * @atomic_disable: disable the plane * * The helper operations are called by the mid-layer CRTC helper. */ @@ -66,6 +67,8 @@ struct drm_plane_helper_funcs { struct drm_plane_state *state); void (*atomic_update)(struct drm_plane *plane, struct drm_plane_state *old_state); + void (*atomic_disable)(struct drm_plane *plane, + struct drm_plane_state *old_state); }; static inline void drm_plane_helper_add(struct drm_plane *plane, -- cgit v1.2.1 From 4cd4df8080a3e0d9b5a75dd52fa2133738def213 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Wed, 3 Dec 2014 16:44:34 +0100 Subject: drm/atomic: Add ->atomic_check() to encoder helpers This callback can be used instead of the legacy ->mode_fixup() and is passed the CRTC and connector states. It can thus use these states to validate the modeset and cache values in the state to be used during the actual modeset. Reviewed-by: Daniel Vetter Signed-off-by: Thierry Reding --- include/drm/drm_crtc_helper.h | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'include') diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h index e76828d81a8b..5810c027acdc 100644 --- a/include/drm/drm_crtc_helper.h +++ b/include/drm/drm_crtc_helper.h @@ -115,6 +115,7 @@ struct drm_crtc_helper_funcs { * @get_crtc: return CRTC that the encoder is currently attached to * @detect: connection status detection * @disable: disable encoder when not in use (overrides DPMS off) + * @atomic_check: check for validity of an atomic update * * The helper operations are called by the mid-layer CRTC helper. */ @@ -137,6 +138,11 @@ struct drm_encoder_helper_funcs { struct drm_connector *connector); /* disable encoder when not in use - more explicit than dpms off */ void (*disable)(struct drm_encoder *encoder); + + /* atomic helpers */ + int (*atomic_check)(struct drm_encoder *encoder, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state); }; /** -- cgit v1.2.1