* [PATCH 1/5] drm: Add drm_dynarray
2017-07-03 13:43 [PATCH 0/5] drm/atomic: drm_dynarray and private obj cleanup ville.syrjala
@ 2017-07-03 13:43 ` ville.syrjala
2017-07-03 13:43 ` [PATCH 2/5] drm/atomic: Convert state->connectors to drm_dynarray ville.syrjala
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: ville.syrjala @ 2017-07-03 13:43 UTC (permalink / raw)
To: dri-devel
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Add a small helper that gives us dynamically growing arrays. We have a
couple hand rolled implementations of this in the atomic code, which we
can unify to use a common implementation.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
Documentation/gpu/drm-utils.rst | 15 +++++++
Documentation/gpu/index.rst | 1 +
drivers/gpu/drm/Makefile | 2 +-
drivers/gpu/drm/drm_dynarray.c | 97 +++++++++++++++++++++++++++++++++++++++++
include/drm/drm_dynarray.h | 54 +++++++++++++++++++++++
5 files changed, 168 insertions(+), 1 deletion(-)
create mode 100644 Documentation/gpu/drm-utils.rst
create mode 100644 drivers/gpu/drm/drm_dynarray.c
create mode 100644 include/drm/drm_dynarray.h
diff --git a/Documentation/gpu/drm-utils.rst b/Documentation/gpu/drm-utils.rst
new file mode 100644
index 000000000000..bff8c899d7cd
--- /dev/null
+++ b/Documentation/gpu/drm-utils.rst
@@ -0,0 +1,15 @@
+=============
+DRM Utilities
+=============
+
+Dynamic arrays
+--------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_dynarray.c
+ :doc: Dynamic arrays
+
+.. kernel-doc:: drivers/gpu/drm/drm_dynarray.c
+ :export:
+
+.. kernel-doc:: include/drm/drm_dynarray.h
+ :internal:
diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst
index 35d673bf9b56..b7d196e5c70d 100644
--- a/Documentation/gpu/index.rst
+++ b/Documentation/gpu/index.rst
@@ -10,6 +10,7 @@ Linux GPU Driver Developer's Guide
drm-kms
drm-kms-helpers
drm-uapi
+ drm-utils
i915
meson
pl111
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 24a066e1841c..b637a34df388 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -3,7 +3,7 @@
# Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
drm-y := drm_auth.o drm_bufs.o drm_cache.o \
- drm_context.o drm_dma.o \
+ drm_context.o drm_dma.o drm_dynarray.o \
drm_file.o drm_gem.o drm_ioctl.o drm_irq.o \
drm_lock.o drm_memory.o drm_drv.o \
drm_scatter.o drm_pci.o \
diff --git a/drivers/gpu/drm/drm_dynarray.c b/drivers/gpu/drm/drm_dynarray.c
new file mode 100644
index 000000000000..69a8819ecb62
--- /dev/null
+++ b/drivers/gpu/drm/drm_dynarray.c
@@ -0,0 +1,97 @@
+/*
+ * Copyright (C) 2017 Intel Corp.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <drm/drm_dynarray.h>
+
+/**
+ * DOC: Dynamic arrays
+ *
+ * Helper that provides dynamically growing arrays. The array
+ * must be initilaized to specify the size of each element, and
+ * space can be reserved in the array by specifying the element
+ * index to be used.
+ */
+
+/**
+ * drm_dynarray_init - Initialize the dynamic array
+ * @dynarr: the dynamic array
+ * @elem_size: size of each element in bytes
+ *
+ * Initialize the dynamic array and specify the size of
+ * each element of the array.
+ */
+void drm_dynarray_init(struct drm_dynarray *dynarr,
+ unsigned int elem_size)
+{
+ memset(dynarr, 0, sizeof(*dynarr));
+ dynarr->elem_size = elem_size;
+}
+EXPORT_SYMBOL(drm_dynarray_init);
+
+/**
+ * drm_dynarray_fini - Finalize the dynamic array
+ * @dynarr: the dynamic array
+ *
+ * Finalize the dynamic array, ie. free the memory
+ * used by the array.
+ */
+void drm_dynarray_fini(struct drm_dynarray *dynarr)
+{
+ kfree(dynarr->elems);
+ memset(dynarr, 0, sizeof(*dynarr));
+}
+EXPORT_SYMBOL(drm_dynarray_fini);
+
+/**
+ * drm_dynarray_reserve - Reserve space in the dynamic array
+ * @dynarr: the dynamic array
+ * @index: the index of the element to reserve
+ *
+ * Grow the array sufficiently to make sure @index points
+ * to a valid memory location within the array.
+ */
+int drm_dynarray_reserve(struct drm_dynarray *dynarr,
+ unsigned int index)
+{
+ unsigned int num_elems = index + 1;
+ unsigned int old_num_elems = dynarr->num_elems;
+ void *elems;
+
+ if (num_elems <= old_num_elems)
+ return 0;
+
+ elems = krealloc(dynarr->elems,
+ num_elems * dynarr->elem_size, GFP_KERNEL);
+ if (!elems)
+ return -ENOMEM;
+
+ dynarr->elems = elems;
+ dynarr->num_elems = num_elems;
+
+ memset(drm_dynarray_elem(dynarr, old_num_elems), 0,
+ (num_elems - old_num_elems) * dynarr->elem_size);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_dynarray_reserve);
diff --git a/include/drm/drm_dynarray.h b/include/drm/drm_dynarray.h
new file mode 100644
index 000000000000..c8cd088a3a3b
--- /dev/null
+++ b/include/drm/drm_dynarray.h
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2017 Intel Corp.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef DRM_DYNARRAY_H
+#define DRM_DYNARRAY_H
+
+struct drm_dynarray {
+ void *elems;
+ unsigned int elem_size, num_elems;
+};
+
+/**
+ * drm_dynarray_elem - Return a pointer to an element
+ * @dynarr: the dynamic array
+ * @index: the index of the element
+ *
+ * Returns:
+ * A pointer to the element at @index in the array.
+ */
+static inline void *drm_dynarray_elem(const struct drm_dynarray *dynarr,
+ unsigned int index)
+{
+ if (index >= dynarr->num_elems)
+ return NULL;
+ return dynarr->elems + index * dynarr->elem_size;
+}
+
+int drm_dynarray_reserve(struct drm_dynarray *dynarr,
+ unsigned int index);
+
+void drm_dynarray_init(struct drm_dynarray *dynarr,
+ unsigned int elem_size);
+void drm_dynarray_fini(struct drm_dynarray *dynarr);
+
+#endif
--
2.13.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/5] drm/atomic: Convert state->connectors to drm_dynarray
2017-07-03 13:43 [PATCH 0/5] drm/atomic: drm_dynarray and private obj cleanup ville.syrjala
2017-07-03 13:43 ` [PATCH 1/5] drm: Add drm_dynarray ville.syrjala
@ 2017-07-03 13:43 ` ville.syrjala
2017-07-03 13:43 ` [PATCH 3/5] drm/atomic: Remove pointless private object NULL state check ville.syrjala
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: ville.syrjala @ 2017-07-03 13:43 UTC (permalink / raw)
To: dri-devel
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
state->connectors[] can grows dynamically, so we can switch over to
using the new drm_dynarray.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_atomic.c | 49 +++++++++++++++++++------------------
drivers/gpu/drm/drm_atomic_helper.c | 4 +--
include/drm/drm_atomic.h | 42 ++++++++++++++++++-------------
3 files changed, 52 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 09ca662fcd35..1663ec3626a1 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -28,6 +28,7 @@
#include <drm/drmP.h>
#include <drm/drm_atomic.h>
+#include <drm/drm_dynarray.h>
#include <drm/drm_mode.h>
#include <drm/drm_print.h>
#include <linux/sync_file.h>
@@ -53,7 +54,7 @@ EXPORT_SYMBOL(__drm_crtc_commit_free);
*/
void drm_atomic_state_default_release(struct drm_atomic_state *state)
{
- kfree(state->connectors);
+ drm_dynarray_fini(&state->connectors);
kfree(state->crtcs);
kfree(state->planes);
kfree(state->private_objs);
@@ -87,6 +88,9 @@ drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state)
if (!state->planes)
goto fail;
+ drm_dynarray_init(&state->connectors,
+ sizeof(struct __drm_connectors_state));
+
state->dev = dev;
DRM_DEBUG_ATOMIC("Allocated atomic state %p\n", state);
@@ -142,15 +146,17 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state)
DRM_DEBUG_ATOMIC("Clearing atomic state %p\n", state);
for (i = 0; i < state->num_connector; i++) {
- struct drm_connector *connector = state->connectors[i].ptr;
+ struct __drm_connectors_state *c =
+ __drm_atomic_state_connector(state, i);
+ struct drm_connector *connector = c->ptr;
if (!connector)
continue;
connector->funcs->atomic_destroy_state(connector,
- state->connectors[i].state);
- state->connectors[i].ptr = NULL;
- state->connectors[i].state = NULL;
+ c->state);
+ c->ptr = NULL;
+ c->state = NULL;
drm_connector_put(connector);
}
@@ -1064,6 +1070,7 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state,
int ret, index;
struct drm_mode_config *config = &connector->dev->mode_config;
struct drm_connector_state *connector_state;
+ struct __drm_connectors_state *c;
WARN_ON(!state->acquire_ctx);
@@ -1073,35 +1080,29 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state,
index = drm_connector_index(connector);
- if (index >= state->num_connector) {
- struct __drm_connnectors_state *c;
- int alloc = max(index + 1, config->num_connector);
-
- c = krealloc(state->connectors, alloc * sizeof(*state->connectors), GFP_KERNEL);
- if (!c)
- return ERR_PTR(-ENOMEM);
-
- state->connectors = c;
- memset(&state->connectors[state->num_connector], 0,
- sizeof(*state->connectors) * (alloc - state->num_connector));
+ ret = drm_dynarray_reserve(&state->connectors,
+ max(index, config->num_connector - 1));
+ if (ret)
+ return ERR_PTR(ret);
- state->num_connector = alloc;
- }
+ c = __drm_atomic_state_connector(state, index);
- if (state->connectors[index].state)
- return state->connectors[index].state;
+ if (c->state)
+ return c->state;
connector_state = connector->funcs->atomic_duplicate_state(connector);
if (!connector_state)
return ERR_PTR(-ENOMEM);
drm_connector_get(connector);
- state->connectors[index].state = connector_state;
- state->connectors[index].old_state = connector->state;
- state->connectors[index].new_state = connector_state;
- state->connectors[index].ptr = connector;
+ c->state = connector_state;
+ c->old_state = connector->state;
+ c->new_state = connector_state;
+ c->ptr = connector;
connector_state->state = state;
+ state->num_connector = state->connectors.num_elems;
+
DRM_DEBUG_ATOMIC("Added [CONNECTOR:%d:%s] %p state to %p\n",
connector->base.id, connector->name,
connector_state, state);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 4f6e52961951..85c96011888f 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2297,7 +2297,7 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
old_conn_state->state = state;
new_conn_state->state = NULL;
- state->connectors[i].state = old_conn_state;
+ __drm_atomic_state_connector(state, i)->state = old_conn_state;
connector->state = new_conn_state;
}
@@ -2871,7 +2871,7 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
state->crtcs[i].old_state = crtc->state;
for_each_new_connector_in_state(state, connector, new_conn_state, i)
- state->connectors[i].old_state = connector->state;
+ __drm_atomic_state_connector(state, i)->old_state = connector->state;
return drm_atomic_commit(state);
}
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index dcc8e0cdb7ff..44316ce45fbb 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -29,6 +29,7 @@
#define DRM_ATOMIC_H_
#include <drm/drm_crtc.h>
+#include <drm/drm_dynarray.h>
/**
* struct drm_crtc_commit - track modeset commits on a CRTC
@@ -149,7 +150,7 @@ struct __drm_crtcs_state {
unsigned last_vblank_count;
};
-struct __drm_connnectors_state {
+struct __drm_connectors_state {
struct drm_connector *ptr;
struct drm_connector_state *state, *old_state, *new_state;
};
@@ -226,7 +227,7 @@ struct drm_atomic_state {
struct __drm_planes_state *planes;
struct __drm_crtcs_state *crtcs;
int num_connector;
- struct __drm_connnectors_state *connectors;
+ struct drm_dynarray connectors; /* struct __drm_connectors_state [] */;
int num_private_objs;
struct __drm_private_objs_state *private_objs;
@@ -241,6 +242,13 @@ struct drm_atomic_state {
struct work_struct commit_work;
};
+static inline struct __drm_connectors_state *
+__drm_atomic_state_connector(const struct drm_atomic_state *state,
+ unsigned int index)
+{
+ return drm_dynarray_elem(&state->connectors, index);
+}
+
void __drm_crtc_commit_free(struct kref *kref);
/**
@@ -441,7 +449,7 @@ drm_atomic_get_existing_connector_state(struct drm_atomic_state *state,
if (index >= state->num_connector)
return NULL;
- return state->connectors[index].state;
+ return __drm_atomic_state_connector(state, index)->state;
}
/**
@@ -461,7 +469,7 @@ drm_atomic_get_old_connector_state(struct drm_atomic_state *state,
if (index >= state->num_connector)
return NULL;
- return state->connectors[index].old_state;
+ return __drm_atomic_state_connector(state, index)->old_state;
}
/**
@@ -481,7 +489,7 @@ drm_atomic_get_new_connector_state(struct drm_atomic_state *state,
if (index >= state->num_connector)
return NULL;
- return state->connectors[index].new_state;
+ return __drm_atomic_state_connector(state, index)->new_state;
}
/**
@@ -573,9 +581,9 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
*/
#define for_each_connector_in_state(__state, connector, connector_state, __i) \
for ((__i) = 0; \
- (__i) < (__state)->num_connector && \
- ((connector) = (__state)->connectors[__i].ptr, \
- (connector_state) = (__state)->connectors[__i].state, 1); \
+ (__i) < (__state)->num_connector && \
+ ((connector) = __drm_atomic_state_connector(__state, __i)->ptr, \
+ (connector_state) = __drm_atomic_state_connector(__state, __i)->state, 1); \
(__i)++) \
for_each_if (connector)
@@ -595,10 +603,10 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
*/
#define for_each_oldnew_connector_in_state(__state, connector, old_connector_state, new_connector_state, __i) \
for ((__i) = 0; \
- (__i) < (__state)->num_connector && \
- ((connector) = (__state)->connectors[__i].ptr, \
- (old_connector_state) = (__state)->connectors[__i].old_state, \
- (new_connector_state) = (__state)->connectors[__i].new_state, 1); \
+ (__i) < (__state)->num_connector && \
+ ((connector) = __drm_atomic_state_connector(__state, __i)->ptr, \
+ (old_connector_state) = __drm_atomic_state_connector(__state, __i)->old_state, \
+ (new_connector_state) = __drm_atomic_state_connector(__state, __i)->new_state, 1); \
(__i)++) \
for_each_if (connector)
@@ -616,9 +624,9 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
*/
#define for_each_old_connector_in_state(__state, connector, old_connector_state, __i) \
for ((__i) = 0; \
- (__i) < (__state)->num_connector && \
- ((connector) = (__state)->connectors[__i].ptr, \
- (old_connector_state) = (__state)->connectors[__i].old_state, 1); \
+ (__i) < (__state)->num_connector && \
+ ((connector) = __drm_atomic_state_connector(__state, __i)->ptr, \
+ (old_connector_state) = __drm_atomic_state_connector(__state, __i)->old_state, 1); \
(__i)++) \
for_each_if (connector)
@@ -637,8 +645,8 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
#define for_each_new_connector_in_state(__state, connector, new_connector_state, __i) \
for ((__i) = 0; \
(__i) < (__state)->num_connector && \
- ((connector) = (__state)->connectors[__i].ptr, \
- (new_connector_state) = (__state)->connectors[__i].new_state, 1); \
+ ((connector) = __drm_atomic_state_connector(__state, __i)->ptr, \
+ (new_connector_state) = __drm_atomic_state_connector(__state, __i)->new_state, 1); \
(__i)++) \
for_each_if (connector)
--
2.13.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/5] drm/atomic: Remove pointless private object NULL state check
2017-07-03 13:43 [PATCH 0/5] drm/atomic: drm_dynarray and private obj cleanup ville.syrjala
2017-07-03 13:43 ` [PATCH 1/5] drm: Add drm_dynarray ville.syrjala
2017-07-03 13:43 ` [PATCH 2/5] drm/atomic: Convert state->connectors to drm_dynarray ville.syrjala
@ 2017-07-03 13:43 ` ville.syrjala
2017-07-03 16:53 ` Daniel Vetter
2017-07-03 13:43 ` [PATCH 4/5] drm/atomic: Convert private_objs to drm_dynarray ville.syrjala
2017-07-03 13:43 ` [PATCH 5/5] drm/atomic: Make private objs proper objects ville.syrjala
4 siblings, 1 reply; 9+ messages in thread
From: ville.syrjala @ 2017-07-03 13:43 UTC (permalink / raw)
To: dri-devel; +Cc: Dhinakaran Pandiyan
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
We will never add private objects with a NULL state into the atomic
state, hence checking for that is pointless.
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_atomic.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 1663ec3626a1..a61e396b11a8 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1019,8 +1019,7 @@ drm_atomic_get_private_obj_state(struct drm_atomic_state *state, void *obj,
struct __drm_private_objs_state *arr;
for (i = 0; i < state->num_private_objs; i++)
- if (obj == state->private_objs[i].obj &&
- state->private_objs[i].obj_state)
+ if (obj == state->private_objs[i].obj)
return state->private_objs[i].obj_state;
num_objs = state->num_private_objs + 1;
--
2.13.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/5] drm/atomic: Remove pointless private object NULL state check
2017-07-03 13:43 ` [PATCH 3/5] drm/atomic: Remove pointless private object NULL state check ville.syrjala
@ 2017-07-03 16:53 ` Daniel Vetter
2017-07-03 17:39 ` Ville Syrjälä
0 siblings, 1 reply; 9+ messages in thread
From: Daniel Vetter @ 2017-07-03 16:53 UTC (permalink / raw)
To: ville.syrjala; +Cc: Dhinakaran Pandiyan, dri-devel
On Mon, Jul 03, 2017 at 04:43:35PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We will never add private objects with a NULL state into the atomic
> state, hence checking for that is pointless.
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
For patches 1&2, feels a bit overengineered to me, I think the other one
was simpler. Not objecting, you just need to find someone else to
like&review it.
-Daniel
> ---
> drivers/gpu/drm/drm_atomic.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 1663ec3626a1..a61e396b11a8 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1019,8 +1019,7 @@ drm_atomic_get_private_obj_state(struct drm_atomic_state *state, void *obj,
> struct __drm_private_objs_state *arr;
>
> for (i = 0; i < state->num_private_objs; i++)
> - if (obj == state->private_objs[i].obj &&
> - state->private_objs[i].obj_state)
> + if (obj == state->private_objs[i].obj)
> return state->private_objs[i].obj_state;
>
> num_objs = state->num_private_objs + 1;
> --
> 2.13.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/5] drm/atomic: Remove pointless private object NULL state check
2017-07-03 16:53 ` Daniel Vetter
@ 2017-07-03 17:39 ` Ville Syrjälä
0 siblings, 0 replies; 9+ messages in thread
From: Ville Syrjälä @ 2017-07-03 17:39 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Dhinakaran Pandiyan, dri-devel
On Mon, Jul 03, 2017 at 06:53:38PM +0200, Daniel Vetter wrote:
> On Mon, Jul 03, 2017 at 04:43:35PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We will never add private objects with a NULL state into the atomic
> > state, hence checking for that is pointless.
> >
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> For patches 1&2, feels a bit overengineered to me, I think the other one
> was simpler.
I had to do the same thing for private objects, hence I figure why not
share the code and avoid putting in slightly different bugs for
different type object.
> Not objecting, you just need to find someone else to
> like&review it.
> -Daniel
>
> > ---
> > drivers/gpu/drm/drm_atomic.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 1663ec3626a1..a61e396b11a8 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -1019,8 +1019,7 @@ drm_atomic_get_private_obj_state(struct drm_atomic_state *state, void *obj,
> > struct __drm_private_objs_state *arr;
> >
> > for (i = 0; i < state->num_private_objs; i++)
> > - if (obj == state->private_objs[i].obj &&
> > - state->private_objs[i].obj_state)
> > + if (obj == state->private_objs[i].obj)
> > return state->private_objs[i].obj_state;
> >
> > num_objs = state->num_private_objs + 1;
> > --
> > 2.13.0
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/5] drm/atomic: Convert private_objs to drm_dynarray
2017-07-03 13:43 [PATCH 0/5] drm/atomic: drm_dynarray and private obj cleanup ville.syrjala
` (2 preceding siblings ...)
2017-07-03 13:43 ` [PATCH 3/5] drm/atomic: Remove pointless private object NULL state check ville.syrjala
@ 2017-07-03 13:43 ` ville.syrjala
2017-07-03 13:43 ` [PATCH 5/5] drm/atomic: Make private objs proper objects ville.syrjala
4 siblings, 0 replies; 9+ messages in thread
From: ville.syrjala @ 2017-07-03 13:43 UTC (permalink / raw)
To: dri-devel; +Cc: Dhinakaran Pandiyan
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
state->private_objs grows dynamically, so switch it over to use
the new drm_dynarray helper.
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_atomic.c | 64 ++++++++++++++++++++-----------------
drivers/gpu/drm/drm_atomic_helper.c | 2 +-
include/drm/drm_atomic.h | 15 ++++++---
3 files changed, 46 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index a61e396b11a8..5eb14c73c0fb 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -57,7 +57,7 @@ void drm_atomic_state_default_release(struct drm_atomic_state *state)
drm_dynarray_fini(&state->connectors);
kfree(state->crtcs);
kfree(state->planes);
- kfree(state->private_objs);
+ drm_dynarray_fini(&state->private_objs);
}
EXPORT_SYMBOL(drm_atomic_state_default_release);
@@ -90,6 +90,8 @@ drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state)
drm_dynarray_init(&state->connectors,
sizeof(struct __drm_connectors_state));
+ drm_dynarray_init(&state->private_objs,
+ sizeof(struct __drm_private_objs_state));
state->dev = dev;
@@ -193,12 +195,14 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state)
}
for (i = 0; i < state->num_private_objs; i++) {
- void *obj_state = state->private_objs[i].obj_state;
-
- state->private_objs[i].funcs->destroy_state(obj_state);
- state->private_objs[i].obj = NULL;
- state->private_objs[i].obj_state = NULL;
- state->private_objs[i].funcs = NULL;
+ struct __drm_private_objs_state *p =
+ __drm_atomic_state_private_obj(state, i);
+ void *obj_state = p->obj_state;
+
+ p->funcs->destroy_state(obj_state);
+ p->obj = NULL;
+ p->obj_state = NULL;
+ p->funcs = NULL;
}
state->num_private_objs = 0;
@@ -1014,36 +1018,36 @@ void *
drm_atomic_get_private_obj_state(struct drm_atomic_state *state, void *obj,
const struct drm_private_state_funcs *funcs)
{
- int index, num_objs, i;
- size_t size;
- struct __drm_private_objs_state *arr;
-
- for (i = 0; i < state->num_private_objs; i++)
- if (obj == state->private_objs[i].obj)
- return state->private_objs[i].obj_state;
-
- num_objs = state->num_private_objs + 1;
- size = sizeof(*state->private_objs) * num_objs;
- arr = krealloc(state->private_objs, size, GFP_KERNEL);
- if (!arr)
- return ERR_PTR(-ENOMEM);
+ struct __drm_private_objs_state *p;
+ int index = state->num_private_objs;
+ int ret, i;
+
+ for (i = 0; i < state->num_private_objs; i++) {
+ p = __drm_atomic_state_private_obj(state, i);
+
+ if (obj == p->obj)
+ return p->obj_state;
+ }
- state->private_objs = arr;
- index = state->num_private_objs;
- memset(&state->private_objs[index], 0, sizeof(*state->private_objs));
+ ret = drm_dynarray_reserve(&state->private_objs, index);
+ if (ret)
+ return ERR_PTR(ret);
- state->private_objs[index].obj_state = funcs->duplicate_state(state, obj);
- if (!state->private_objs[index].obj_state)
+ p = __drm_atomic_state_private_obj(state, index);
+
+ p->obj_state = funcs->duplicate_state(state, obj);
+ if (!p->obj_state)
return ERR_PTR(-ENOMEM);
- state->private_objs[index].obj = obj;
- state->private_objs[index].funcs = funcs;
- state->num_private_objs = num_objs;
+ p->obj = obj;
+ p->funcs = funcs;
+
+ state->num_private_objs = index + 1;
DRM_DEBUG_ATOMIC("Added new private object state %p to %p\n",
- state->private_objs[index].obj_state, state);
+ p->obj_state, state);
- return state->private_objs[index].obj_state;
+ return p->obj_state;
}
EXPORT_SYMBOL(drm_atomic_get_private_obj_state);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 85c96011888f..5cd93c6d691e 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2331,7 +2331,7 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
}
__for_each_private_obj(state, obj, obj_state, i, funcs)
- funcs->swap_state(obj, &state->private_objs[i].obj_state);
+ funcs->swap_state(obj, &__drm_atomic_state_private_obj(state, i)->obj_state);
}
EXPORT_SYMBOL(drm_atomic_helper_swap_state);
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 44316ce45fbb..809e8b4c3719 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -229,7 +229,7 @@ struct drm_atomic_state {
int num_connector;
struct drm_dynarray connectors; /* struct __drm_connectors_state [] */;
int num_private_objs;
- struct __drm_private_objs_state *private_objs;
+ struct drm_dynarray private_objs; /* struct __drm_private_objs_state [] */;
struct drm_modeset_acquire_ctx *acquire_ctx;
@@ -249,6 +249,13 @@ __drm_atomic_state_connector(const struct drm_atomic_state *state,
return drm_dynarray_elem(&state->connectors, index);
}
+static inline struct __drm_private_objs_state *
+__drm_atomic_state_private_obj(const struct drm_atomic_state *state,
+ unsigned int index)
+{
+ return drm_dynarray_elem(&state->private_objs, index);
+}
+
void __drm_crtc_commit_free(struct kref *kref);
/**
@@ -832,9 +839,9 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
#define __for_each_private_obj(__state, obj, obj_state, __i, __funcs) \
for ((__i) = 0; \
(__i) < (__state)->num_private_objs && \
- ((obj) = (__state)->private_objs[__i].obj, \
- (__funcs) = (__state)->private_objs[__i].funcs, \
- (obj_state) = (__state)->private_objs[__i].obj_state, \
+ ((obj) = __drm_atomic_state_private_obj(__state, __i)->obj, \
+ (__funcs) = __drm_atomic_state_private_obj(__state, __i)->funcs, \
+ (obj_state) = __drm_atomic_state_private_obj(__state, __i)->obj_state, \
1); \
(__i)++) \
--
2.13.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 5/5] drm/atomic: Make private objs proper objects
2017-07-03 13:43 [PATCH 0/5] drm/atomic: drm_dynarray and private obj cleanup ville.syrjala
` (3 preceding siblings ...)
2017-07-03 13:43 ` [PATCH 4/5] drm/atomic: Convert private_objs to drm_dynarray ville.syrjala
@ 2017-07-03 13:43 ` ville.syrjala
2017-07-03 17:00 ` Daniel Vetter
4 siblings, 1 reply; 9+ messages in thread
From: ville.syrjala @ 2017-07-03 13:43 UTC (permalink / raw)
To: dri-devel; +Cc: Dhinakaran Pandiyan
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Make the atomic private object stuff less special by introducing proper
base classes for the object and its state. Drivers can embed these in
their own appropriate objects, after which these things will work
exactly like the plane/crtc/connector states during atomic operations.
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_atomic.c | 78 +++++++++++++++------
drivers/gpu/drm/drm_atomic_helper.c | 30 +++++++--
drivers/gpu/drm/drm_dp_mst_topology.c | 63 +++++++++--------
include/drm/drm_atomic.h | 123 +++++++++++++++++++++-------------
include/drm/drm_atomic_helper.h | 4 ++
include/drm/drm_dp_mst_helper.h | 10 +++
6 files changed, 206 insertions(+), 102 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 5eb14c73c0fb..da7752230e4c 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -197,12 +197,14 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state)
for (i = 0; i < state->num_private_objs; i++) {
struct __drm_private_objs_state *p =
__drm_atomic_state_private_obj(state, i);
- void *obj_state = p->obj_state;
+ struct drm_private_obj *obj = p->ptr;
- p->funcs->destroy_state(obj_state);
- p->obj = NULL;
- p->obj_state = NULL;
- p->funcs = NULL;
+ if (WARN_ON(!obj))
+ continue;
+
+ obj->funcs->atomic_destroy_state(obj, p->state);
+ p->ptr = NULL;
+ p->state = NULL;
}
state->num_private_objs = 0;
@@ -1000,11 +1002,44 @@ static void drm_atomic_plane_print_state(struct drm_printer *p,
}
/**
+ * drm_atomic_private_obj_init - initialize private object
+ * @obj: private object
+ * @state: initial private object state
+ * @funcs: pointer to the struct of function pointers that identify the object
+ * type
+ *
+ * Initialize the private object, which can be embedded into any
+ * driver private object that needs its own atomic state.
+ */
+void
+drm_atomic_private_obj_init(struct drm_private_obj *obj,
+ struct drm_private_state *state,
+ const struct drm_private_state_funcs *funcs)
+{
+ memset(obj, 0, sizeof(*obj));
+
+ obj->state = state;
+ obj->funcs = funcs;
+}
+EXPORT_SYMBOL(drm_atomic_private_obj_init);
+
+/**
+ * drm_atomic_private_obj_fini - finalize private object
+ * @obj: private object
+ *
+ * Finalize the private object.
+ */
+void
+drm_atomic_private_obj_fini(struct drm_private_obj *obj)
+{
+ obj->funcs->atomic_destroy_state(obj, obj->state);
+}
+EXPORT_SYMBOL(drm_atomic_private_obj_fini);
+
+/**
* drm_atomic_get_private_obj_state - get private object state
* @state: global atomic state
* @obj: private object to get the state for
- * @funcs: pointer to the struct of function pointers that identify the object
- * type
*
* This function returns the private object state for the given private object,
* allocating the state if needed. It does not grab any locks as the caller is
@@ -1014,19 +1049,21 @@ static void drm_atomic_plane_print_state(struct drm_printer *p,
*
* Either the allocated state or the error code encoded into a pointer.
*/
-void *
-drm_atomic_get_private_obj_state(struct drm_atomic_state *state, void *obj,
- const struct drm_private_state_funcs *funcs)
+struct drm_private_state *
+drm_atomic_get_private_obj_state(struct drm_atomic_state *state,
+ struct drm_private_obj *obj)
{
+ const struct drm_private_state_funcs *funcs = obj->funcs;
struct __drm_private_objs_state *p;
+ struct drm_private_state *obj_state;
int index = state->num_private_objs;
int ret, i;
for (i = 0; i < state->num_private_objs; i++) {
p = __drm_atomic_state_private_obj(state, i);
- if (obj == p->obj)
- return p->obj_state;
+ if (obj == p->ptr)
+ return p->state;
}
ret = drm_dynarray_reserve(&state->private_objs, index);
@@ -1035,19 +1072,22 @@ drm_atomic_get_private_obj_state(struct drm_atomic_state *state, void *obj,
p = __drm_atomic_state_private_obj(state, index);
- p->obj_state = funcs->duplicate_state(state, obj);
- if (!p->obj_state)
+ obj_state = funcs->atomic_duplicate_state(obj);
+ if (!obj_state)
return ERR_PTR(-ENOMEM);
- p->obj = obj;
- p->funcs = funcs;
+ p->state = obj_state;
+ p->old_state = obj->state;
+ p->new_state = obj_state;
+ p->ptr = obj;
+ obj_state->state = state;
state->num_private_objs = index + 1;
- DRM_DEBUG_ATOMIC("Added new private object state %p to %p\n",
- p->obj_state, state);
+ DRM_DEBUG_ATOMIC("Added new private object [%p] state %p to %p\n",
+ obj, obj_state, state);
- return p->obj_state;
+ return obj_state;
}
EXPORT_SYMBOL(drm_atomic_get_private_obj_state);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 5cd93c6d691e..78f0ff1ad982 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2267,8 +2267,8 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
struct drm_plane *plane;
struct drm_plane_state *old_plane_state, *new_plane_state;
struct drm_crtc_commit *commit;
- void *obj, *obj_state;
- const struct drm_private_state_funcs *funcs;
+ struct drm_private_obj *obj;
+ struct drm_private_state *old_obj_state, *new_obj_state;
if (stall) {
for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
@@ -2330,8 +2330,15 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
plane->state = new_plane_state;
}
- __for_each_private_obj(state, obj, obj_state, i, funcs)
- funcs->swap_state(obj, &__drm_atomic_state_private_obj(state, i)->obj_state);
+ for_each_oldnew_private_obj_in_state(state, obj, old_obj_state, new_obj_state, i) {
+ WARN_ON(obj->state != old_obj_state);
+
+ old_obj_state->state = state;
+ new_obj_state->state = NULL;
+
+ __drm_atomic_state_private_obj(state, i)->state = old_obj_state;
+ obj->state = new_obj_state;
+ }
}
EXPORT_SYMBOL(drm_atomic_helper_swap_state);
@@ -3830,3 +3837,18 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
return ret;
}
EXPORT_SYMBOL(drm_atomic_helper_legacy_gamma_set);
+
+/**
+ * __drm_atomic_helper_private_duplicate_state - copy atomic private state
+ * @obj: CRTC object
+ * @state: new private object state
+ *
+ * Copies atomic state from a private objects's current state and resets inferred values.
+ * This is useful for drivers that subclass the private state.
+ */
+void __drm_atomic_helper_private_obj_duplicate_state(struct drm_private_obj *obj,
+ struct drm_private_state *state)
+{
+ memcpy(state, obj->state, sizeof(*state));
+}
+EXPORT_SYMBOL(__drm_atomic_helper_private_obj_duplicate_state);
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index bfd237c15e76..91510098f60e 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -31,6 +31,8 @@
#include <drm/drmP.h>
#include <drm/drm_fixed.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
/**
* DOC: dp mst helper
@@ -2992,41 +2994,32 @@ static void drm_dp_destroy_connector_work(struct work_struct *work)
(*mgr->cbs->hotplug)(mgr);
}
-void *drm_dp_mst_duplicate_state(struct drm_atomic_state *state, void *obj)
+static struct drm_private_state *
+drm_dp_mst_duplicate_state(struct drm_private_obj *obj)
{
- struct drm_dp_mst_topology_mgr *mgr = obj;
- struct drm_dp_mst_topology_state *new_mst_state;
+ struct drm_dp_mst_topology_state *state;
- if (WARN_ON(!mgr->state))
+ state = kmemdup(obj->state, sizeof(*state), GFP_KERNEL);
+ if (!state)
return NULL;
- new_mst_state = kmemdup(mgr->state, sizeof(*new_mst_state), GFP_KERNEL);
- if (new_mst_state)
- new_mst_state->state = state;
- return new_mst_state;
-}
-
-void drm_dp_mst_swap_state(void *obj, void **obj_state_ptr)
-{
- struct drm_dp_mst_topology_mgr *mgr = obj;
- struct drm_dp_mst_topology_state **topology_state_ptr;
-
- topology_state_ptr = (struct drm_dp_mst_topology_state **)obj_state_ptr;
+ __drm_atomic_helper_private_obj_duplicate_state(obj, &state->base);
- mgr->state->state = (*topology_state_ptr)->state;
- swap(*topology_state_ptr, mgr->state);
- mgr->state->state = NULL;
+ return &state->base;
}
-void drm_dp_mst_destroy_state(void *obj_state)
+static void drm_dp_mst_destroy_state(struct drm_private_obj *obj,
+ struct drm_private_state *state)
{
- kfree(obj_state);
+ struct drm_dp_mst_topology_state *mst_state =
+ to_dp_mst_topology_state(state);
+
+ kfree(mst_state);
}
static const struct drm_private_state_funcs mst_state_funcs = {
- .duplicate_state = drm_dp_mst_duplicate_state,
- .swap_state = drm_dp_mst_swap_state,
- .destroy_state = drm_dp_mst_destroy_state,
+ .atomic_duplicate_state = drm_dp_mst_duplicate_state,
+ .atomic_destroy_state = drm_dp_mst_destroy_state,
};
/**
@@ -3050,8 +3043,7 @@ struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct drm_a
struct drm_device *dev = mgr->dev;
WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
- return drm_atomic_get_private_obj_state(state, mgr,
- &mst_state_funcs);
+ return to_dp_mst_topology_state(drm_atomic_get_private_obj_state(state, &mgr->base));
}
EXPORT_SYMBOL(drm_atomic_get_mst_topology_state);
@@ -3071,6 +3063,8 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
int max_dpcd_transaction_bytes,
int max_payloads, int conn_base_id)
{
+ struct drm_dp_mst_topology_state *mst_state;
+
mutex_init(&mgr->lock);
mutex_init(&mgr->qlock);
mutex_init(&mgr->payload_lock);
@@ -3099,14 +3093,18 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
if (test_calc_pbn_mode() < 0)
DRM_ERROR("MST PBN self-test failed\n");
- mgr->state = kzalloc(sizeof(*mgr->state), GFP_KERNEL);
- if (mgr->state == NULL)
+ mst_state = kzalloc(sizeof(*mst_state), GFP_KERNEL);
+ if (mst_state == NULL)
return -ENOMEM;
- mgr->state->mgr = mgr;
+
+ mst_state->mgr = mgr;
/* max. time slots - one slot for MTP header */
- mgr->state->avail_slots = 63;
- mgr->funcs = &mst_state_funcs;
+ mst_state->avail_slots = 63;
+
+ drm_atomic_private_obj_init(&mgr->base,
+ &mst_state->base,
+ &mst_state_funcs);
return 0;
}
@@ -3128,8 +3126,7 @@ void drm_dp_mst_topology_mgr_destroy(struct drm_dp_mst_topology_mgr *mgr)
mutex_unlock(&mgr->payload_lock);
mgr->dev = NULL;
mgr->aux = NULL;
- kfree(mgr->state);
- mgr->state = NULL;
+ drm_atomic_private_obj_fini(&mgr->base);
mgr->funcs = NULL;
}
EXPORT_SYMBOL(drm_dp_mst_topology_mgr_destroy);
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 809e8b4c3719..addec49a14bf 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -155,6 +155,9 @@ struct __drm_connectors_state {
struct drm_connector_state *state, *old_state, *new_state;
};
+struct drm_private_obj;
+struct drm_private_state;
+
/**
* struct drm_private_state_funcs - atomic state functions for private objects
*
@@ -167,7 +170,7 @@ struct __drm_connectors_state {
*/
struct drm_private_state_funcs {
/**
- * @duplicate_state:
+ * @atomic_duplicate_state:
*
* Duplicate the current state of the private object and return it. It
* is an error to call this before obj->state has been initialized.
@@ -177,29 +180,30 @@ struct drm_private_state_funcs {
* Duplicated atomic state or NULL when obj->state is not
* initialized or allocation failed.
*/
- void *(*duplicate_state)(struct drm_atomic_state *state, void *obj);
+ struct drm_private_state *(*atomic_duplicate_state)(struct drm_private_obj *obj);
/**
- * @swap_state:
+ * @atomic_destroy_state:
*
- * This function swaps the existing state of a private object @obj with
- * it's newly created state, the pointer to which is passed as
- * @obj_state_ptr.
+ * Frees the private object state created with @atomic_duplicate_state.
*/
- void (*swap_state)(void *obj, void **obj_state_ptr);
+ void (*atomic_destroy_state)(struct drm_private_obj *obj,
+ struct drm_private_state *state);
+};
- /**
- * @destroy_state:
- *
- * Frees the private object state created with @duplicate_state.
- */
- void (*destroy_state)(void *obj_state);
+struct drm_private_obj {
+ struct drm_private_state *state;
+
+ const struct drm_private_state_funcs *funcs;
+};
+
+struct drm_private_state {
+ struct drm_atomic_state *state;
};
struct __drm_private_objs_state {
- void *obj;
- void *obj_state;
- const struct drm_private_state_funcs *funcs;
+ struct drm_private_obj *ptr;
+ struct drm_private_state *state, *old_state, *new_state;
};
/**
@@ -336,10 +340,14 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
struct drm_connector_state *state, struct drm_property *property,
uint64_t val);
-void * __must_check
+void drm_atomic_private_obj_init(struct drm_private_obj *obj,
+ struct drm_private_state *state,
+ const struct drm_private_state_funcs *funcs);
+void drm_atomic_private_obj_fini(struct drm_private_obj *obj);
+
+struct drm_private_state * __must_check
drm_atomic_get_private_obj_state(struct drm_atomic_state *state,
- void *obj,
- const struct drm_private_state_funcs *funcs);
+ struct drm_private_obj *obj);
/**
* drm_atomic_get_existing_crtc_state - get crtc state, if it exists
@@ -826,43 +834,66 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
for_each_if (plane)
/**
- * __for_each_private_obj - iterate over all private objects
+ * for_each_oldnew_private_obj_in_state - iterate over all private objects in an atomic update
* @__state: &struct drm_atomic_state pointer
- * @obj: private object iteration cursor
- * @obj_state: private object state iteration cursor
+ * @obj: &struct drm_private_obj iteration cursor
+ * @old_obj_state: &struct drm_private_state iteration cursor for the old state
+ * @new_obj_state: &struct drm_private_state iteration cursor for the new state
* @__i: int iteration cursor, for macro-internal use
- * @__funcs: &struct drm_private_state_funcs iteration cursor
*
- * This macro iterates over the array containing private object data in atomic
- * state
+ * This iterates over all private objects in an atomic update, tracking both
+ * old and new state. This is useful in places where the state delta needs
+ * to be considered, for example in atomic check functions.
*/
-#define __for_each_private_obj(__state, obj, obj_state, __i, __funcs) \
- for ((__i) = 0; \
- (__i) < (__state)->num_private_objs && \
- ((obj) = __drm_atomic_state_private_obj(__state, __i)->obj, \
- (__funcs) = __drm_atomic_state_private_obj(__state, __i)->funcs, \
- (obj_state) = __drm_atomic_state_private_obj(__state, __i)->obj_state, \
- 1); \
- (__i)++) \
+#define for_each_oldnew_private_obj_in_state(__state, obj, old_obj_state, new_obj_state, __i) \
+ for ((__i) = 0; \
+ (__i) < (__state)->num_private_objs && \
+ ((obj) = __drm_atomic_state_private_obj(__state, __i)->ptr, \
+ (old_obj_state) = __drm_atomic_state_private_obj(__state, __i)->old_state, \
+ (new_obj_state) = __drm_atomic_state_private_obj(__state, __i)->new_state, 1); \
+ (__i)++) \
+ for_each_if (obj)
+
/**
- * for_each_private_obj - iterate over a specify type of private object
+ * for_each_old_private_obj_in_state - iterate over all private objects in an atomic update
* @__state: &struct drm_atomic_state pointer
- * @obj_funcs: &struct drm_private_state_funcs function table to filter
- * private objects
- * @obj: private object iteration cursor
- * @obj_state: private object state iteration cursor
+ * @obj: &struct drm_private_obj iteration cursor
+ * @old_obj_state: &struct drm_private_state iteration cursor for the old state
* @__i: int iteration cursor, for macro-internal use
- * @__funcs: &struct drm_private_state_funcs iteration cursor
*
- * This macro iterates over the private objects state array while filtering the
- * objects based on the vfunc table that is passed as @obj_funcs. New macros
- * can be created by passing in the vfunc table associated with a specific
- * private object.
+ * This iterates over all private objects in an atomic update, tracking only
+ * the old state. This is useful in disable functions, where we need the old
+ * state the hardware is still in.
*/
-#define for_each_private_obj(__state, obj_funcs, obj, obj_state, __i, __funcs) \
- __for_each_private_obj(__state, obj, obj_state, __i, __funcs) \
- for_each_if (__funcs == obj_funcs)
+#define for_each_old_private_obj_in_state(__state, obj, old_obj_state, __i) \
+ for ((__i) = 0; \
+ (__i) < (__state)->num_private_objs && \
+ ((obj) = __drm_atomic_state_private_obj(__state, __i)->ptr, \
+ (old_obj_state) = __drm_atomic_state_private_obj(__state, __i)->old_state, 1); \
+ (__i)++) \
+ for_each_if (obj)
+
+
+/**
+ * for_each_new_private_obj_in_state - iterate over all private objects in an atomic update
+ * @__state: &struct drm_atomic_state pointer
+ * @obj: &struct drm_private_obj iteration cursor
+ * @new_obj_state: &struct drm_private_state iteration cursor for the new state
+ * @__i: int iteration cursor, for macro-internal use
+ *
+ * This iterates over all private objects in an atomic update, tracking only
+ * the new state. This is useful in enable functions, where we need the new state the
+ * hardware should be in when the atomic commit operation has completed.
+ */
+#define for_each_new_private_obj_in_state(__state, obj, new_obj_state, __i) \
+ for ((__i) = 0; \
+ (__i) < (__state)->num_private_objs && \
+ ((obj) = __drm_atomic_state_private_obj(__state, __i)->ptr, \
+ (new_obj_state) = __drm_atomic_state_private_obj(__state, __i)->new_state, 1); \
+ (__i)++) \
+ for_each_if (obj)
+
/**
* drm_atomic_crtc_needs_modeset - compute combined modeset need
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index dd196cc0afd7..7db3438ff735 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -33,6 +33,8 @@
#include <drm/drm_modeset_helper.h>
struct drm_atomic_state;
+struct drm_private_obj;
+struct drm_private_state;
int drm_atomic_helper_check_modeset(struct drm_device *dev,
struct drm_atomic_state *state);
@@ -185,6 +187,8 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
u16 *red, u16 *green, u16 *blue,
uint32_t size,
struct drm_modeset_acquire_ctx *ctx);
+void __drm_atomic_helper_private_obj_duplicate_state(struct drm_private_obj *obj,
+ struct drm_private_state *state);
/**
* drm_atomic_crtc_for_each_plane - iterate over planes currently attached to CRTC
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 177ab6f86855..d55abb75f29a 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -404,12 +404,17 @@ struct drm_dp_payload {
int vcpi;
};
+#define to_dp_mst_topology_state(x) container_of(x, struct drm_dp_mst_topology_state, base)
+
struct drm_dp_mst_topology_state {
+ struct drm_private_state base;
int avail_slots;
struct drm_atomic_state *state;
struct drm_dp_mst_topology_mgr *mgr;
};
+#define to_dp_mst_topology_mgr(x) container_of(x, struct drm_dp_mst_topology_mgr, base)
+
/**
* struct drm_dp_mst_topology_mgr - DisplayPort MST manager
*
@@ -419,6 +424,11 @@ struct drm_dp_mst_topology_state {
*/
struct drm_dp_mst_topology_mgr {
/**
+ * @base: Base private object for atomic
+ */
+ struct drm_private_obj base;
+
+ /**
* @dev: device pointer for adding i2c devices etc.
*/
struct drm_device *dev;
--
2.13.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 5/5] drm/atomic: Make private objs proper objects
2017-07-03 13:43 ` [PATCH 5/5] drm/atomic: Make private objs proper objects ville.syrjala
@ 2017-07-03 17:00 ` Daniel Vetter
0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2017-07-03 17:00 UTC (permalink / raw)
To: ville.syrjala; +Cc: Dhinakaran Pandiyan, dri-devel
On Mon, Jul 03, 2017 at 04:43:37PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Make the atomic private object stuff less special by introducing proper
> base classes for the object and its state. Drivers can embed these in
> their own appropriate objects, after which these things will work
> exactly like the plane/crtc/connector states during atomic operations.
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Won't apply without 1&4, but glossing over that detail I kinda like this.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/drm_atomic.c | 78 +++++++++++++++------
> drivers/gpu/drm/drm_atomic_helper.c | 30 +++++++--
> drivers/gpu/drm/drm_dp_mst_topology.c | 63 +++++++++--------
> include/drm/drm_atomic.h | 123 +++++++++++++++++++++-------------
> include/drm/drm_atomic_helper.h | 4 ++
> include/drm/drm_dp_mst_helper.h | 10 +++
> 6 files changed, 206 insertions(+), 102 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 5eb14c73c0fb..da7752230e4c 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -197,12 +197,14 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state)
> for (i = 0; i < state->num_private_objs; i++) {
> struct __drm_private_objs_state *p =
> __drm_atomic_state_private_obj(state, i);
> - void *obj_state = p->obj_state;
> + struct drm_private_obj *obj = p->ptr;
>
> - p->funcs->destroy_state(obj_state);
> - p->obj = NULL;
> - p->obj_state = NULL;
> - p->funcs = NULL;
> + if (WARN_ON(!obj))
> + continue;
> +
> + obj->funcs->atomic_destroy_state(obj, p->state);
> + p->ptr = NULL;
> + p->state = NULL;
> }
> state->num_private_objs = 0;
>
> @@ -1000,11 +1002,44 @@ static void drm_atomic_plane_print_state(struct drm_printer *p,
> }
>
> /**
> + * drm_atomic_private_obj_init - initialize private object
> + * @obj: private object
> + * @state: initial private object state
> + * @funcs: pointer to the struct of function pointers that identify the object
> + * type
> + *
> + * Initialize the private object, which can be embedded into any
> + * driver private object that needs its own atomic state.
> + */
> +void
> +drm_atomic_private_obj_init(struct drm_private_obj *obj,
> + struct drm_private_state *state,
> + const struct drm_private_state_funcs *funcs)
> +{
> + memset(obj, 0, sizeof(*obj));
> +
> + obj->state = state;
> + obj->funcs = funcs;
> +}
> +EXPORT_SYMBOL(drm_atomic_private_obj_init);
> +
> +/**
> + * drm_atomic_private_obj_fini - finalize private object
> + * @obj: private object
> + *
> + * Finalize the private object.
> + */
> +void
> +drm_atomic_private_obj_fini(struct drm_private_obj *obj)
> +{
> + obj->funcs->atomic_destroy_state(obj, obj->state);
> +}
> +EXPORT_SYMBOL(drm_atomic_private_obj_fini);
> +
> +/**
> * drm_atomic_get_private_obj_state - get private object state
> * @state: global atomic state
> * @obj: private object to get the state for
> - * @funcs: pointer to the struct of function pointers that identify the object
> - * type
> *
> * This function returns the private object state for the given private object,
> * allocating the state if needed. It does not grab any locks as the caller is
> @@ -1014,19 +1049,21 @@ static void drm_atomic_plane_print_state(struct drm_printer *p,
> *
> * Either the allocated state or the error code encoded into a pointer.
> */
> -void *
> -drm_atomic_get_private_obj_state(struct drm_atomic_state *state, void *obj,
> - const struct drm_private_state_funcs *funcs)
> +struct drm_private_state *
> +drm_atomic_get_private_obj_state(struct drm_atomic_state *state,
> + struct drm_private_obj *obj)
> {
> + const struct drm_private_state_funcs *funcs = obj->funcs;
> struct __drm_private_objs_state *p;
> + struct drm_private_state *obj_state;
> int index = state->num_private_objs;
> int ret, i;
>
> for (i = 0; i < state->num_private_objs; i++) {
> p = __drm_atomic_state_private_obj(state, i);
>
> - if (obj == p->obj)
> - return p->obj_state;
> + if (obj == p->ptr)
> + return p->state;
> }
>
> ret = drm_dynarray_reserve(&state->private_objs, index);
> @@ -1035,19 +1072,22 @@ drm_atomic_get_private_obj_state(struct drm_atomic_state *state, void *obj,
>
> p = __drm_atomic_state_private_obj(state, index);
>
> - p->obj_state = funcs->duplicate_state(state, obj);
> - if (!p->obj_state)
> + obj_state = funcs->atomic_duplicate_state(obj);
> + if (!obj_state)
> return ERR_PTR(-ENOMEM);
>
> - p->obj = obj;
> - p->funcs = funcs;
> + p->state = obj_state;
> + p->old_state = obj->state;
> + p->new_state = obj_state;
> + p->ptr = obj;
> + obj_state->state = state;
>
> state->num_private_objs = index + 1;
>
> - DRM_DEBUG_ATOMIC("Added new private object state %p to %p\n",
> - p->obj_state, state);
> + DRM_DEBUG_ATOMIC("Added new private object [%p] state %p to %p\n",
> + obj, obj_state, state);
>
> - return p->obj_state;
> + return obj_state;
> }
> EXPORT_SYMBOL(drm_atomic_get_private_obj_state);
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 5cd93c6d691e..78f0ff1ad982 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2267,8 +2267,8 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
> struct drm_plane *plane;
> struct drm_plane_state *old_plane_state, *new_plane_state;
> struct drm_crtc_commit *commit;
> - void *obj, *obj_state;
> - const struct drm_private_state_funcs *funcs;
> + struct drm_private_obj *obj;
> + struct drm_private_state *old_obj_state, *new_obj_state;
>
> if (stall) {
> for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> @@ -2330,8 +2330,15 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
> plane->state = new_plane_state;
> }
>
> - __for_each_private_obj(state, obj, obj_state, i, funcs)
> - funcs->swap_state(obj, &__drm_atomic_state_private_obj(state, i)->obj_state);
> + for_each_oldnew_private_obj_in_state(state, obj, old_obj_state, new_obj_state, i) {
> + WARN_ON(obj->state != old_obj_state);
> +
> + old_obj_state->state = state;
> + new_obj_state->state = NULL;
> +
> + __drm_atomic_state_private_obj(state, i)->state = old_obj_state;
> + obj->state = new_obj_state;
> + }
> }
> EXPORT_SYMBOL(drm_atomic_helper_swap_state);
>
> @@ -3830,3 +3837,18 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> return ret;
> }
> EXPORT_SYMBOL(drm_atomic_helper_legacy_gamma_set);
> +
> +/**
> + * __drm_atomic_helper_private_duplicate_state - copy atomic private state
> + * @obj: CRTC object
> + * @state: new private object state
> + *
> + * Copies atomic state from a private objects's current state and resets inferred values.
> + * This is useful for drivers that subclass the private state.
> + */
> +void __drm_atomic_helper_private_obj_duplicate_state(struct drm_private_obj *obj,
> + struct drm_private_state *state)
> +{
> + memcpy(state, obj->state, sizeof(*state));
> +}
> +EXPORT_SYMBOL(__drm_atomic_helper_private_obj_duplicate_state);
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index bfd237c15e76..91510098f60e 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -31,6 +31,8 @@
> #include <drm/drmP.h>
>
> #include <drm/drm_fixed.h>
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_atomic_helper.h>
>
> /**
> * DOC: dp mst helper
> @@ -2992,41 +2994,32 @@ static void drm_dp_destroy_connector_work(struct work_struct *work)
> (*mgr->cbs->hotplug)(mgr);
> }
>
> -void *drm_dp_mst_duplicate_state(struct drm_atomic_state *state, void *obj)
> +static struct drm_private_state *
> +drm_dp_mst_duplicate_state(struct drm_private_obj *obj)
> {
> - struct drm_dp_mst_topology_mgr *mgr = obj;
> - struct drm_dp_mst_topology_state *new_mst_state;
> + struct drm_dp_mst_topology_state *state;
>
> - if (WARN_ON(!mgr->state))
> + state = kmemdup(obj->state, sizeof(*state), GFP_KERNEL);
> + if (!state)
> return NULL;
>
> - new_mst_state = kmemdup(mgr->state, sizeof(*new_mst_state), GFP_KERNEL);
> - if (new_mst_state)
> - new_mst_state->state = state;
> - return new_mst_state;
> -}
> -
> -void drm_dp_mst_swap_state(void *obj, void **obj_state_ptr)
> -{
> - struct drm_dp_mst_topology_mgr *mgr = obj;
> - struct drm_dp_mst_topology_state **topology_state_ptr;
> -
> - topology_state_ptr = (struct drm_dp_mst_topology_state **)obj_state_ptr;
> + __drm_atomic_helper_private_obj_duplicate_state(obj, &state->base);
>
> - mgr->state->state = (*topology_state_ptr)->state;
> - swap(*topology_state_ptr, mgr->state);
> - mgr->state->state = NULL;
> + return &state->base;
> }
>
> -void drm_dp_mst_destroy_state(void *obj_state)
> +static void drm_dp_mst_destroy_state(struct drm_private_obj *obj,
> + struct drm_private_state *state)
> {
> - kfree(obj_state);
> + struct drm_dp_mst_topology_state *mst_state =
> + to_dp_mst_topology_state(state);
> +
> + kfree(mst_state);
> }
>
> static const struct drm_private_state_funcs mst_state_funcs = {
> - .duplicate_state = drm_dp_mst_duplicate_state,
> - .swap_state = drm_dp_mst_swap_state,
> - .destroy_state = drm_dp_mst_destroy_state,
> + .atomic_duplicate_state = drm_dp_mst_duplicate_state,
> + .atomic_destroy_state = drm_dp_mst_destroy_state,
> };
>
> /**
> @@ -3050,8 +3043,7 @@ struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct drm_a
> struct drm_device *dev = mgr->dev;
>
> WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
> - return drm_atomic_get_private_obj_state(state, mgr,
> - &mst_state_funcs);
> + return to_dp_mst_topology_state(drm_atomic_get_private_obj_state(state, &mgr->base));
> }
> EXPORT_SYMBOL(drm_atomic_get_mst_topology_state);
>
> @@ -3071,6 +3063,8 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
> int max_dpcd_transaction_bytes,
> int max_payloads, int conn_base_id)
> {
> + struct drm_dp_mst_topology_state *mst_state;
> +
> mutex_init(&mgr->lock);
> mutex_init(&mgr->qlock);
> mutex_init(&mgr->payload_lock);
> @@ -3099,14 +3093,18 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
> if (test_calc_pbn_mode() < 0)
> DRM_ERROR("MST PBN self-test failed\n");
>
> - mgr->state = kzalloc(sizeof(*mgr->state), GFP_KERNEL);
> - if (mgr->state == NULL)
> + mst_state = kzalloc(sizeof(*mst_state), GFP_KERNEL);
> + if (mst_state == NULL)
> return -ENOMEM;
> - mgr->state->mgr = mgr;
> +
> + mst_state->mgr = mgr;
>
> /* max. time slots - one slot for MTP header */
> - mgr->state->avail_slots = 63;
> - mgr->funcs = &mst_state_funcs;
> + mst_state->avail_slots = 63;
> +
> + drm_atomic_private_obj_init(&mgr->base,
> + &mst_state->base,
> + &mst_state_funcs);
>
> return 0;
> }
> @@ -3128,8 +3126,7 @@ void drm_dp_mst_topology_mgr_destroy(struct drm_dp_mst_topology_mgr *mgr)
> mutex_unlock(&mgr->payload_lock);
> mgr->dev = NULL;
> mgr->aux = NULL;
> - kfree(mgr->state);
> - mgr->state = NULL;
> + drm_atomic_private_obj_fini(&mgr->base);
> mgr->funcs = NULL;
> }
> EXPORT_SYMBOL(drm_dp_mst_topology_mgr_destroy);
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 809e8b4c3719..addec49a14bf 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -155,6 +155,9 @@ struct __drm_connectors_state {
> struct drm_connector_state *state, *old_state, *new_state;
> };
>
> +struct drm_private_obj;
> +struct drm_private_state;
> +
> /**
> * struct drm_private_state_funcs - atomic state functions for private objects
> *
> @@ -167,7 +170,7 @@ struct __drm_connectors_state {
> */
> struct drm_private_state_funcs {
> /**
> - * @duplicate_state:
> + * @atomic_duplicate_state:
> *
> * Duplicate the current state of the private object and return it. It
> * is an error to call this before obj->state has been initialized.
> @@ -177,29 +180,30 @@ struct drm_private_state_funcs {
> * Duplicated atomic state or NULL when obj->state is not
> * initialized or allocation failed.
> */
> - void *(*duplicate_state)(struct drm_atomic_state *state, void *obj);
> + struct drm_private_state *(*atomic_duplicate_state)(struct drm_private_obj *obj);
>
> /**
> - * @swap_state:
> + * @atomic_destroy_state:
> *
> - * This function swaps the existing state of a private object @obj with
> - * it's newly created state, the pointer to which is passed as
> - * @obj_state_ptr.
> + * Frees the private object state created with @atomic_duplicate_state.
> */
> - void (*swap_state)(void *obj, void **obj_state_ptr);
> + void (*atomic_destroy_state)(struct drm_private_obj *obj,
> + struct drm_private_state *state);
> +};
>
> - /**
> - * @destroy_state:
> - *
> - * Frees the private object state created with @duplicate_state.
> - */
> - void (*destroy_state)(void *obj_state);
> +struct drm_private_obj {
> + struct drm_private_state *state;
> +
> + const struct drm_private_state_funcs *funcs;
> +};
> +
> +struct drm_private_state {
> + struct drm_atomic_state *state;
> };
>
> struct __drm_private_objs_state {
> - void *obj;
> - void *obj_state;
> - const struct drm_private_state_funcs *funcs;
> + struct drm_private_obj *ptr;
> + struct drm_private_state *state, *old_state, *new_state;
> };
>
> /**
> @@ -336,10 +340,14 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
> struct drm_connector_state *state, struct drm_property *property,
> uint64_t val);
>
> -void * __must_check
> +void drm_atomic_private_obj_init(struct drm_private_obj *obj,
> + struct drm_private_state *state,
> + const struct drm_private_state_funcs *funcs);
> +void drm_atomic_private_obj_fini(struct drm_private_obj *obj);
> +
> +struct drm_private_state * __must_check
> drm_atomic_get_private_obj_state(struct drm_atomic_state *state,
> - void *obj,
> - const struct drm_private_state_funcs *funcs);
> + struct drm_private_obj *obj);
>
> /**
> * drm_atomic_get_existing_crtc_state - get crtc state, if it exists
> @@ -826,43 +834,66 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
> for_each_if (plane)
>
> /**
> - * __for_each_private_obj - iterate over all private objects
> + * for_each_oldnew_private_obj_in_state - iterate over all private objects in an atomic update
> * @__state: &struct drm_atomic_state pointer
> - * @obj: private object iteration cursor
> - * @obj_state: private object state iteration cursor
> + * @obj: &struct drm_private_obj iteration cursor
> + * @old_obj_state: &struct drm_private_state iteration cursor for the old state
> + * @new_obj_state: &struct drm_private_state iteration cursor for the new state
> * @__i: int iteration cursor, for macro-internal use
> - * @__funcs: &struct drm_private_state_funcs iteration cursor
> *
> - * This macro iterates over the array containing private object data in atomic
> - * state
> + * This iterates over all private objects in an atomic update, tracking both
> + * old and new state. This is useful in places where the state delta needs
> + * to be considered, for example in atomic check functions.
> */
> -#define __for_each_private_obj(__state, obj, obj_state, __i, __funcs) \
> - for ((__i) = 0; \
> - (__i) < (__state)->num_private_objs && \
> - ((obj) = __drm_atomic_state_private_obj(__state, __i)->obj, \
> - (__funcs) = __drm_atomic_state_private_obj(__state, __i)->funcs, \
> - (obj_state) = __drm_atomic_state_private_obj(__state, __i)->obj_state, \
> - 1); \
> - (__i)++) \
> +#define for_each_oldnew_private_obj_in_state(__state, obj, old_obj_state, new_obj_state, __i) \
> + for ((__i) = 0; \
> + (__i) < (__state)->num_private_objs && \
> + ((obj) = __drm_atomic_state_private_obj(__state, __i)->ptr, \
> + (old_obj_state) = __drm_atomic_state_private_obj(__state, __i)->old_state, \
> + (new_obj_state) = __drm_atomic_state_private_obj(__state, __i)->new_state, 1); \
> + (__i)++) \
> + for_each_if (obj)
> +
>
> /**
> - * for_each_private_obj - iterate over a specify type of private object
> + * for_each_old_private_obj_in_state - iterate over all private objects in an atomic update
> * @__state: &struct drm_atomic_state pointer
> - * @obj_funcs: &struct drm_private_state_funcs function table to filter
> - * private objects
> - * @obj: private object iteration cursor
> - * @obj_state: private object state iteration cursor
> + * @obj: &struct drm_private_obj iteration cursor
> + * @old_obj_state: &struct drm_private_state iteration cursor for the old state
> * @__i: int iteration cursor, for macro-internal use
> - * @__funcs: &struct drm_private_state_funcs iteration cursor
> *
> - * This macro iterates over the private objects state array while filtering the
> - * objects based on the vfunc table that is passed as @obj_funcs. New macros
> - * can be created by passing in the vfunc table associated with a specific
> - * private object.
> + * This iterates over all private objects in an atomic update, tracking only
> + * the old state. This is useful in disable functions, where we need the old
> + * state the hardware is still in.
> */
> -#define for_each_private_obj(__state, obj_funcs, obj, obj_state, __i, __funcs) \
> - __for_each_private_obj(__state, obj, obj_state, __i, __funcs) \
> - for_each_if (__funcs == obj_funcs)
> +#define for_each_old_private_obj_in_state(__state, obj, old_obj_state, __i) \
> + for ((__i) = 0; \
> + (__i) < (__state)->num_private_objs && \
> + ((obj) = __drm_atomic_state_private_obj(__state, __i)->ptr, \
> + (old_obj_state) = __drm_atomic_state_private_obj(__state, __i)->old_state, 1); \
> + (__i)++) \
> + for_each_if (obj)
> +
> +
> +/**
> + * for_each_new_private_obj_in_state - iterate over all private objects in an atomic update
> + * @__state: &struct drm_atomic_state pointer
> + * @obj: &struct drm_private_obj iteration cursor
> + * @new_obj_state: &struct drm_private_state iteration cursor for the new state
> + * @__i: int iteration cursor, for macro-internal use
> + *
> + * This iterates over all private objects in an atomic update, tracking only
> + * the new state. This is useful in enable functions, where we need the new state the
> + * hardware should be in when the atomic commit operation has completed.
> + */
> +#define for_each_new_private_obj_in_state(__state, obj, new_obj_state, __i) \
> + for ((__i) = 0; \
> + (__i) < (__state)->num_private_objs && \
> + ((obj) = __drm_atomic_state_private_obj(__state, __i)->ptr, \
> + (new_obj_state) = __drm_atomic_state_private_obj(__state, __i)->new_state, 1); \
> + (__i)++) \
> + for_each_if (obj)
> +
>
> /**
> * drm_atomic_crtc_needs_modeset - compute combined modeset need
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index dd196cc0afd7..7db3438ff735 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -33,6 +33,8 @@
> #include <drm/drm_modeset_helper.h>
>
> struct drm_atomic_state;
> +struct drm_private_obj;
> +struct drm_private_state;
>
> int drm_atomic_helper_check_modeset(struct drm_device *dev,
> struct drm_atomic_state *state);
> @@ -185,6 +187,8 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> u16 *red, u16 *green, u16 *blue,
> uint32_t size,
> struct drm_modeset_acquire_ctx *ctx);
> +void __drm_atomic_helper_private_obj_duplicate_state(struct drm_private_obj *obj,
> + struct drm_private_state *state);
>
> /**
> * drm_atomic_crtc_for_each_plane - iterate over planes currently attached to CRTC
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index 177ab6f86855..d55abb75f29a 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -404,12 +404,17 @@ struct drm_dp_payload {
> int vcpi;
> };
>
> +#define to_dp_mst_topology_state(x) container_of(x, struct drm_dp_mst_topology_state, base)
> +
> struct drm_dp_mst_topology_state {
> + struct drm_private_state base;
> int avail_slots;
> struct drm_atomic_state *state;
> struct drm_dp_mst_topology_mgr *mgr;
> };
>
> +#define to_dp_mst_topology_mgr(x) container_of(x, struct drm_dp_mst_topology_mgr, base)
> +
> /**
> * struct drm_dp_mst_topology_mgr - DisplayPort MST manager
> *
> @@ -419,6 +424,11 @@ struct drm_dp_mst_topology_state {
> */
> struct drm_dp_mst_topology_mgr {
> /**
> + * @base: Base private object for atomic
> + */
> + struct drm_private_obj base;
> +
> + /**
> * @dev: device pointer for adding i2c devices etc.
> */
> struct drm_device *dev;
> --
> 2.13.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread