Drivers may need to store the state of shared resources, such as PLLs or FIFO space, into the atomic state. Allow this by making it possible to subclass drm_atomic_state.
Changes since v1: - Change member names for functions to atomic_state_(alloc,clear) - Change __drm_atomic_state_new to drm_atomic_state_init - Allow free function to be overridden too, in case extra memory is allocated in alloc.
Cc: dri-devel@lists.freedesktop.org Acked-by: Ander Conselvan de Oliveira ander.conselvan.de.oliveira@intel.com Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com --- drivers/gpu/drm/drm_atomic.c | 116 ++++++++++++++++++++++++++++++++----------- include/drm/drm_atomic.h | 5 ++ include/drm/drm_crtc.h | 6 +++ 3 files changed, 99 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index c6277a4a1f2f..47364f244dc0 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -30,7 +30,15 @@ #include <drm/drm_atomic.h> #include <drm/drm_plane_helper.h>
-static void kfree_state(struct drm_atomic_state *state) +/** + * drm_atomic_state_default_free - + * free memory initialized by drm_atomic_state_init + * @state: atomic state + * + * Free all the memory allocated by drm_atomic_state_init. + * This is useful for drivers that subclass the atomic state. + */ +void drm_atomic_state_default_free(struct drm_atomic_state *state) { kfree(state->connectors); kfree(state->connector_states); @@ -38,24 +46,20 @@ static void kfree_state(struct drm_atomic_state *state) kfree(state->crtc_states); kfree(state->planes); kfree(state->plane_states); - kfree(state); } +EXPORT_SYMBOL(drm_atomic_state_default_free);
/** - * drm_atomic_state_alloc - allocate atomic state + * drm_atomic_state_init - init new atomic state * @dev: DRM device + * @state: atomic state * - * This allocates an empty atomic state to track updates. + * Default implementation for filling in a new atomic state. + * This is useful for drivers that subclass the atomic state. */ -struct drm_atomic_state * -drm_atomic_state_alloc(struct drm_device *dev) +int +drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state) { - struct drm_atomic_state *state; - - state = kzalloc(sizeof(*state), GFP_KERNEL); - if (!state) - return NULL; - /* TODO legacy paths should maybe do a better job about * setting this appropriately? */ @@ -92,31 +96,50 @@ drm_atomic_state_alloc(struct drm_device *dev)
state->dev = dev;
- DRM_DEBUG_ATOMIC("Allocate atomic state %p\n", state); + DRM_DEBUG_ATOMIC("Allocated atomic state %p\n", state);
- return state; + return 0; fail: - kfree_state(state); + drm_atomic_state_default_free(state); + return -ENOMEM; +} +EXPORT_SYMBOL(drm_atomic_state_init); + +/** + * drm_atomic_state_alloc - allocate atomic state + * @dev: DRM device + * + * This allocates an empty atomic state to track updates. + */ +struct drm_atomic_state * +drm_atomic_state_alloc(struct drm_device *dev) +{ + struct drm_mode_config *config = &dev->mode_config; + struct drm_atomic_state *state; + + if (!config->funcs->atomic_state_alloc) { + state = kzalloc(sizeof(*state), GFP_KERNEL); + if (!state) + return NULL; + if (drm_atomic_state_init(dev, state) < 0) { + kfree(state); + return NULL; + } + return state; + }
- return NULL; + return config->funcs->atomic_state_alloc(dev); } EXPORT_SYMBOL(drm_atomic_state_alloc);
/** - * drm_atomic_state_clear - clear state object + * drm_atomic_state_default_clear - clear base atomic state * @state: atomic state * - * When the w/w mutex algorithm detects a deadlock we need to back off and drop - * all locks. So someone else could sneak in and change the current modeset - * configuration. Which means that all the state assembled in @state is no - * longer an atomic update to the current state, but to some arbitrary earlier - * state. Which could break assumptions the driver's ->atomic_check likely - * relies on. - * - * Hence we must clear all cached state and completely start over, using this - * function. + * Default implementation for clearing atomic state. + * This is useful for drivers that subclass the atomic state. */ -void drm_atomic_state_clear(struct drm_atomic_state *state) +void drm_atomic_state_default_clear(struct drm_atomic_state *state) { struct drm_device *dev = state->dev; struct drm_mode_config *config = &dev->mode_config; @@ -162,6 +185,32 @@ void drm_atomic_state_clear(struct drm_atomic_state *state) state->plane_states[i] = NULL; } } +EXPORT_SYMBOL(drm_atomic_state_default_clear); + +/** + * drm_atomic_state_clear - clear state object + * @state: atomic state + * + * When the w/w mutex algorithm detects a deadlock we need to back off and drop + * all locks. So someone else could sneak in and change the current modeset + * configuration. Which means that all the state assembled in @state is no + * longer an atomic update to the current state, but to some arbitrary earlier + * state. Which could break assumptions the driver's ->atomic_check likely + * relies on. + * + * Hence we must clear all cached state and completely start over, using this + * function. + */ +void drm_atomic_state_clear(struct drm_atomic_state *state) +{ + struct drm_device *dev = state->dev; + struct drm_mode_config *config = &dev->mode_config; + + if (config->funcs->atomic_state_clear) + config->funcs->atomic_state_clear(state); + else + drm_atomic_state_default_clear(state); +} EXPORT_SYMBOL(drm_atomic_state_clear);
/** @@ -173,14 +222,25 @@ EXPORT_SYMBOL(drm_atomic_state_clear); */ void drm_atomic_state_free(struct drm_atomic_state *state) { + struct drm_device *dev; + struct drm_mode_config *config; + if (!state) return;
+ dev = state->dev; + config = &dev->mode_config; + drm_atomic_state_clear(state);
DRM_DEBUG_ATOMIC("Freeing atomic state %p\n", state);
- kfree_state(state); + if (config->funcs->atomic_state_free) { + config->funcs->atomic_state_free(state); + } else { + drm_atomic_state_default_free(state); + kfree(state); + } } EXPORT_SYMBOL(drm_atomic_state_free);
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index d78543067700..6445970535ec 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -35,6 +35,11 @@ drm_atomic_state_alloc(struct drm_device *dev); void drm_atomic_state_clear(struct drm_atomic_state *state); void drm_atomic_state_free(struct drm_atomic_state *state);
+int __must_check +drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state); +void drm_atomic_state_default_clear(struct drm_atomic_state *state); +void drm_atomic_state_default_free(struct drm_atomic_state *state); + struct drm_crtc_state * __must_check drm_atomic_get_crtc_state(struct drm_atomic_state *state, struct drm_crtc *crtc); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 840e9e62878c..bff25b0cada9 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -985,6 +985,9 @@ struct drm_mode_set { * @atomic_check: check whether a given atomic state update is possible * @atomic_commit: commit an atomic state update previously verified with * atomic_check() + * @atomic_state_alloc: allocate a new atomic state + * @atomic_state_clear: clear the atomic state + * @atomic_state_free: free the atomic state * * Some global (i.e. not per-CRTC, connector, etc) mode setting functions that * involve drivers. @@ -1000,6 +1003,9 @@ struct drm_mode_config_funcs { int (*atomic_commit)(struct drm_device *dev, struct drm_atomic_state *a, bool async); + struct drm_atomic_state *(*atomic_state_alloc)(struct drm_device *dev); + void (*atomic_state_clear)(struct drm_atomic_state *state); + void (*atomic_state_free)(struct drm_atomic_state *state); };
/**
Drivers may need to store the state of shared resources, such as PLLs or FIFO space, into the atomic state. Allow this by making it possible to subclass drm_atomic_state.
Changes since v1: - Change member names for functions to atomic_state_(alloc,clear) - Change __drm_atomic_state_new to drm_atomic_state_init - Allow free function to be overridden too, in case extra memory is allocated in alloc. Changes since v2: - Rename *_default_free to default_release, to make clear it doesn't free the state object itself.
Cc: dri-devel@lists.freedesktop.org Acked-by: Ander Conselvan de Oliveira ander.conselvan.de.oliveira@intel.com Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index c6277a4a1f2f..cd1b16b25716 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -30,7 +30,15 @@ #include <drm/drm_atomic.h> #include <drm/drm_plane_helper.h>
-static void kfree_state(struct drm_atomic_state *state) +/** + * drm_atomic_state_default_release - + * release memory initialized by drm_atomic_state_init + * @state: atomic state + * + * Free all the memory allocated by drm_atomic_state_init. + * This is useful for drivers that subclass the atomic state. + */ +void drm_atomic_state_default_release(struct drm_atomic_state *state) { kfree(state->connectors); kfree(state->connector_states); @@ -38,24 +46,20 @@ static void kfree_state(struct drm_atomic_state *state) kfree(state->crtc_states); kfree(state->planes); kfree(state->plane_states); - kfree(state); } +EXPORT_SYMBOL(drm_atomic_state_default_release);
/** - * drm_atomic_state_alloc - allocate atomic state + * drm_atomic_state_init - init new atomic state * @dev: DRM device + * @state: atomic state * - * This allocates an empty atomic state to track updates. + * Default implementation for filling in a new atomic state. + * This is useful for drivers that subclass the atomic state. */ -struct drm_atomic_state * -drm_atomic_state_alloc(struct drm_device *dev) +int +drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state) { - struct drm_atomic_state *state; - - state = kzalloc(sizeof(*state), GFP_KERNEL); - if (!state) - return NULL; - /* TODO legacy paths should maybe do a better job about * setting this appropriately? */ @@ -92,31 +96,50 @@ drm_atomic_state_alloc(struct drm_device *dev)
state->dev = dev;
- DRM_DEBUG_ATOMIC("Allocate atomic state %p\n", state); + DRM_DEBUG_ATOMIC("Allocated atomic state %p\n", state);
- return state; + return 0; fail: - kfree_state(state); + drm_atomic_state_default_release(state); + return -ENOMEM; +} +EXPORT_SYMBOL(drm_atomic_state_init); + +/** + * drm_atomic_state_alloc - allocate atomic state + * @dev: DRM device + * + * This allocates an empty atomic state to track updates. + */ +struct drm_atomic_state * +drm_atomic_state_alloc(struct drm_device *dev) +{ + struct drm_mode_config *config = &dev->mode_config; + struct drm_atomic_state *state; + + if (!config->funcs->atomic_state_alloc) { + state = kzalloc(sizeof(*state), GFP_KERNEL); + if (!state) + return NULL; + if (drm_atomic_state_init(dev, state) < 0) { + kfree(state); + return NULL; + } + return state; + }
- return NULL; + return config->funcs->atomic_state_alloc(dev); } EXPORT_SYMBOL(drm_atomic_state_alloc);
/** - * drm_atomic_state_clear - clear state object + * drm_atomic_state_default_clear - clear base atomic state * @state: atomic state * - * When the w/w mutex algorithm detects a deadlock we need to back off and drop - * all locks. So someone else could sneak in and change the current modeset - * configuration. Which means that all the state assembled in @state is no - * longer an atomic update to the current state, but to some arbitrary earlier - * state. Which could break assumptions the driver's ->atomic_check likely - * relies on. - * - * Hence we must clear all cached state and completely start over, using this - * function. + * Default implementation for clearing atomic state. + * This is useful for drivers that subclass the atomic state. */ -void drm_atomic_state_clear(struct drm_atomic_state *state) +void drm_atomic_state_default_clear(struct drm_atomic_state *state) { struct drm_device *dev = state->dev; struct drm_mode_config *config = &dev->mode_config; @@ -162,6 +185,32 @@ void drm_atomic_state_clear(struct drm_atomic_state *state) state->plane_states[i] = NULL; } } +EXPORT_SYMBOL(drm_atomic_state_default_clear); + +/** + * drm_atomic_state_clear - clear state object + * @state: atomic state + * + * When the w/w mutex algorithm detects a deadlock we need to back off and drop + * all locks. So someone else could sneak in and change the current modeset + * configuration. Which means that all the state assembled in @state is no + * longer an atomic update to the current state, but to some arbitrary earlier + * state. Which could break assumptions the driver's ->atomic_check likely + * relies on. + * + * Hence we must clear all cached state and completely start over, using this + * function. + */ +void drm_atomic_state_clear(struct drm_atomic_state *state) +{ + struct drm_device *dev = state->dev; + struct drm_mode_config *config = &dev->mode_config; + + if (config->funcs->atomic_state_clear) + config->funcs->atomic_state_clear(state); + else + drm_atomic_state_default_clear(state); +} EXPORT_SYMBOL(drm_atomic_state_clear);
/** @@ -173,14 +222,25 @@ EXPORT_SYMBOL(drm_atomic_state_clear); */ void drm_atomic_state_free(struct drm_atomic_state *state) { + struct drm_device *dev; + struct drm_mode_config *config; + if (!state) return;
+ dev = state->dev; + config = &dev->mode_config; + drm_atomic_state_clear(state);
DRM_DEBUG_ATOMIC("Freeing atomic state %p\n", state);
- kfree_state(state); + if (config->funcs->atomic_state_free) { + config->funcs->atomic_state_free(state); + } else { + drm_atomic_state_default_release(state); + kfree(state); + } } EXPORT_SYMBOL(drm_atomic_state_free);
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index d78543067700..f0d3a7387d99 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -35,6 +35,11 @@ drm_atomic_state_alloc(struct drm_device *dev); void drm_atomic_state_clear(struct drm_atomic_state *state); void drm_atomic_state_free(struct drm_atomic_state *state);
+int __must_check +drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state); +void drm_atomic_state_default_clear(struct drm_atomic_state *state); +void drm_atomic_state_default_release(struct drm_atomic_state *state); + struct drm_crtc_state * __must_check drm_atomic_get_crtc_state(struct drm_atomic_state *state, struct drm_crtc *crtc); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 840e9e62878c..bff25b0cada9 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -985,6 +985,9 @@ struct drm_mode_set { * @atomic_check: check whether a given atomic state update is possible * @atomic_commit: commit an atomic state update previously verified with * atomic_check() + * @atomic_state_alloc: allocate a new atomic state + * @atomic_state_clear: clear the atomic state + * @atomic_state_free: free the atomic state * * Some global (i.e. not per-CRTC, connector, etc) mode setting functions that * involve drivers. @@ -1000,6 +1003,9 @@ struct drm_mode_config_funcs { int (*atomic_commit)(struct drm_device *dev, struct drm_atomic_state *a, bool async); + struct drm_atomic_state *(*atomic_state_alloc)(struct drm_device *dev); + void (*atomic_state_clear)(struct drm_atomic_state *state); + void (*atomic_state_free)(struct drm_atomic_state *state); };
/**
On Mon, May 18, 2015 at 10:06:40AM +0200, Maarten Lankhorst wrote:
Drivers may need to store the state of shared resources, such as PLLs or FIFO space, into the atomic state. Allow this by making it possible to subclass drm_atomic_state.
Changes since v1:
- Change member names for functions to atomic_state_(alloc,clear)
- Change __drm_atomic_state_new to drm_atomic_state_init
- Allow free function to be overridden too, in case extra memory is allocated in alloc.
Changes since v2:
- Rename *_default_free to default_release, to make clear it doesn't free the state object itself.
I'd have gone with just _release since we don't have a ->release hook nor a drm_atomic_helper_state_release wrapper. But that's really a bikeshed now ;-)
Cc: dri-devel@lists.freedesktop.org Acked-by: Ander Conselvan de Oliveira ander.conselvan.de.oliveira@intel.com Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com
Applied to topic/drm-misc, thanks. -Daniel
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index c6277a4a1f2f..cd1b16b25716 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -30,7 +30,15 @@ #include <drm/drm_atomic.h> #include <drm/drm_plane_helper.h>
-static void kfree_state(struct drm_atomic_state *state) +/**
- drm_atomic_state_default_release -
- release memory initialized by drm_atomic_state_init
- @state: atomic state
- Free all the memory allocated by drm_atomic_state_init.
- This is useful for drivers that subclass the atomic state.
- */
+void drm_atomic_state_default_release(struct drm_atomic_state *state) { kfree(state->connectors); kfree(state->connector_states); @@ -38,24 +46,20 @@ static void kfree_state(struct drm_atomic_state *state) kfree(state->crtc_states); kfree(state->planes); kfree(state->plane_states);
- kfree(state);
} +EXPORT_SYMBOL(drm_atomic_state_default_release);
/**
- drm_atomic_state_alloc - allocate atomic state
- drm_atomic_state_init - init new atomic state
- @dev: DRM device
- @state: atomic state
- This allocates an empty atomic state to track updates.
- Default implementation for filling in a new atomic state.
*/
- This is useful for drivers that subclass the atomic state.
-struct drm_atomic_state * -drm_atomic_state_alloc(struct drm_device *dev) +int +drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state) {
- struct drm_atomic_state *state;
- state = kzalloc(sizeof(*state), GFP_KERNEL);
- if (!state)
return NULL;- /* TODO legacy paths should maybe do a better job about
*/
- setting this appropriately?
@@ -92,31 +96,50 @@ drm_atomic_state_alloc(struct drm_device *dev)
state->dev = dev;
- DRM_DEBUG_ATOMIC("Allocate atomic state %p\n", state);
- DRM_DEBUG_ATOMIC("Allocated atomic state %p\n", state);
- return state;
- return 0;
fail:
- kfree_state(state);
- drm_atomic_state_default_release(state);
- return -ENOMEM;
+} +EXPORT_SYMBOL(drm_atomic_state_init);
+/**
- drm_atomic_state_alloc - allocate atomic state
- @dev: DRM device
- This allocates an empty atomic state to track updates.
- */
+struct drm_atomic_state * +drm_atomic_state_alloc(struct drm_device *dev) +{
- struct drm_mode_config *config = &dev->mode_config;
- struct drm_atomic_state *state;
- if (!config->funcs->atomic_state_alloc) {
state = kzalloc(sizeof(*state), GFP_KERNEL);if (!state)return NULL;if (drm_atomic_state_init(dev, state) < 0) {kfree(state);return NULL;}return state;- }
- return NULL;
- return config->funcs->atomic_state_alloc(dev);
} EXPORT_SYMBOL(drm_atomic_state_alloc);
/**
- drm_atomic_state_clear - clear state object
- drm_atomic_state_default_clear - clear base atomic state
- @state: atomic state
- When the w/w mutex algorithm detects a deadlock we need to back off and drop
- all locks. So someone else could sneak in and change the current modeset
- configuration. Which means that all the state assembled in @state is no
- longer an atomic update to the current state, but to some arbitrary earlier
- state. Which could break assumptions the driver's ->atomic_check likely
- relies on.
- Hence we must clear all cached state and completely start over, using this
- function.
- Default implementation for clearing atomic state.
*/
- This is useful for drivers that subclass the atomic state.
-void drm_atomic_state_clear(struct drm_atomic_state *state) +void drm_atomic_state_default_clear(struct drm_atomic_state *state) { struct drm_device *dev = state->dev; struct drm_mode_config *config = &dev->mode_config; @@ -162,6 +185,32 @@ void drm_atomic_state_clear(struct drm_atomic_state *state) state->plane_states[i] = NULL; } } +EXPORT_SYMBOL(drm_atomic_state_default_clear);
+/**
- drm_atomic_state_clear - clear state object
- @state: atomic state
- When the w/w mutex algorithm detects a deadlock we need to back off and drop
- all locks. So someone else could sneak in and change the current modeset
- configuration. Which means that all the state assembled in @state is no
- longer an atomic update to the current state, but to some arbitrary earlier
- state. Which could break assumptions the driver's ->atomic_check likely
- relies on.
- Hence we must clear all cached state and completely start over, using this
- function.
- */
+void drm_atomic_state_clear(struct drm_atomic_state *state) +{
- struct drm_device *dev = state->dev;
- struct drm_mode_config *config = &dev->mode_config;
- if (config->funcs->atomic_state_clear)
config->funcs->atomic_state_clear(state);- else
drm_atomic_state_default_clear(state);+} EXPORT_SYMBOL(drm_atomic_state_clear);
/** @@ -173,14 +222,25 @@ EXPORT_SYMBOL(drm_atomic_state_clear); */ void drm_atomic_state_free(struct drm_atomic_state *state) {
struct drm_device *dev;
struct drm_mode_config *config;
if (!state) return;
dev = state->dev;
config = &dev->mode_config;
drm_atomic_state_clear(state);
DRM_DEBUG_ATOMIC("Freeing atomic state %p\n", state);
- kfree_state(state);
- if (config->funcs->atomic_state_free) {
config->funcs->atomic_state_free(state);- } else {
drm_atomic_state_default_release(state);kfree(state);- }
} EXPORT_SYMBOL(drm_atomic_state_free);
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index d78543067700..f0d3a7387d99 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -35,6 +35,11 @@ drm_atomic_state_alloc(struct drm_device *dev); void drm_atomic_state_clear(struct drm_atomic_state *state); void drm_atomic_state_free(struct drm_atomic_state *state);
+int __must_check +drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state); +void drm_atomic_state_default_clear(struct drm_atomic_state *state); +void drm_atomic_state_default_release(struct drm_atomic_state *state);
struct drm_crtc_state * __must_check drm_atomic_get_crtc_state(struct drm_atomic_state *state, struct drm_crtc *crtc); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 840e9e62878c..bff25b0cada9 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -985,6 +985,9 @@ struct drm_mode_set {
- @atomic_check: check whether a given atomic state update is possible
- @atomic_commit: commit an atomic state update previously verified with
- atomic_check()
- @atomic_state_alloc: allocate a new atomic state
- @atomic_state_clear: clear the atomic state
- @atomic_state_free: free the atomic state
- Some global (i.e. not per-CRTC, connector, etc) mode setting functions that
- involve drivers.
@@ -1000,6 +1003,9 @@ struct drm_mode_config_funcs { int (*atomic_commit)(struct drm_device *dev, struct drm_atomic_state *a, bool async);
- struct drm_atomic_state *(*atomic_state_alloc)(struct drm_device *dev);
- void (*atomic_state_clear)(struct drm_atomic_state *state);
- void (*atomic_state_free)(struct drm_atomic_state *state);
};
/**
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Maarten Lankhorst