* [PATCH] drm/atomic: use connector references (v3)
@ 2016-05-03 20:03 Dave Airlie
2016-05-03 20:09 ` Daniel Vetter
0 siblings, 1 reply; 3+ messages in thread
From: Dave Airlie @ 2016-05-03 20:03 UTC (permalink / raw)
To: dri-devel, intel-gfx
From: Dave Airlie <airlied@redhat.com>
Take a reference when setting a crtc on a connecter,
also take one when duplicating if a crtc is set,
and drop one on destroy if a crtc is set.
v2: take Daniel Stone's advice and simplify the
ref/unref dances, also take care of NULL as connector
to state reset.
v3: remove need for connector NULL check.
Signed-off-by: Dave Airlie <airlied@redhat.com>
---
drivers/gpu/drm/drm_atomic.c | 14 +++++---------
drivers/gpu/drm/drm_atomic_helper.c | 4 ++++
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 9d5e3c8..0df87a5 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -142,15 +142,7 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state)
if (!connector)
continue;
- /*
- * FIXME: Async commits can race with connector unplugging and
- * there's currently nothing that prevents cleanup up state for
- * deleted connectors. As long as the callback doesn't look at
- * the connector we'll be fine though, so make sure that's the
- * case by setting all connector pointers to NULL.
- */
- state->connector_states[i]->connector = NULL;
- connector->funcs->atomic_destroy_state(NULL,
+ connector->funcs->atomic_destroy_state(connector,
state->connector_states[i]);
state->connectors[i] = NULL;
state->connector_states[i] = NULL;
@@ -1160,6 +1152,8 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
{
struct drm_crtc_state *crtc_state;
+ if (crtc)
+ drm_connector_reference(conn_state->connector);
if (conn_state->crtc && conn_state->crtc != crtc) {
crtc_state = drm_atomic_get_existing_crtc_state(conn_state->state,
conn_state->crtc);
@@ -1177,6 +1171,8 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
1 << drm_connector_index(conn_state->connector);
}
+ if (conn_state->crtc)
+ drm_connector_unreference(conn_state->connector);
conn_state->crtc = crtc;
if (crtc)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index d25abce..c48446d 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2762,6 +2762,8 @@ __drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector,
struct drm_connector_state *state)
{
memcpy(state, connector->state, sizeof(*state));
+ if (state->crtc)
+ drm_connector_reference(connector);
}
EXPORT_SYMBOL(__drm_atomic_helper_connector_duplicate_state);
@@ -2889,6 +2891,8 @@ __drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
* state will automatically do the right thing if code is ever added
* to this function.
*/
+ if (state->crtc)
+ drm_connector_unreference(state->connector);
}
EXPORT_SYMBOL(__drm_atomic_helper_connector_destroy_state);
--
2.5.5
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/atomic: use connector references (v3)
2016-05-03 20:03 [PATCH] drm/atomic: use connector references (v3) Dave Airlie
@ 2016-05-03 20:09 ` Daniel Vetter
2016-05-03 20:13 ` Daniel Stone
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Vetter @ 2016-05-03 20:09 UTC (permalink / raw)
To: Dave Airlie; +Cc: intel-gfx, dri-devel
On Wed, May 04, 2016 at 06:03:05AM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
>
> Take a reference when setting a crtc on a connecter,
> also take one when duplicating if a crtc is set,
> and drop one on destroy if a crtc is set.
>
> v2: take Daniel Stone's advice and simplify the
> ref/unref dances, also take care of NULL as connector
> to state reset.
>
> v3: remove need for connector NULL check.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
This patch series seems cursed with lots of embarrassement for reviewers.
I totally missed/forgot the FIXME in drm_atomic_state_default_clear(),
which this patch fixes. Anyway, I'm happy to burn my hands again ;-)
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/drm_atomic.c | 14 +++++---------
> drivers/gpu/drm/drm_atomic_helper.c | 4 ++++
> 2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 9d5e3c8..0df87a5 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -142,15 +142,7 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state)
> if (!connector)
> continue;
>
> - /*
> - * FIXME: Async commits can race with connector unplugging and
> - * there's currently nothing that prevents cleanup up state for
> - * deleted connectors. As long as the callback doesn't look at
> - * the connector we'll be fine though, so make sure that's the
> - * case by setting all connector pointers to NULL.
> - */
> - state->connector_states[i]->connector = NULL;
> - connector->funcs->atomic_destroy_state(NULL,
> + connector->funcs->atomic_destroy_state(connector,
> state->connector_states[i]);
> state->connectors[i] = NULL;
> state->connector_states[i] = NULL;
> @@ -1160,6 +1152,8 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
> {
> struct drm_crtc_state *crtc_state;
>
> + if (crtc)
> + drm_connector_reference(conn_state->connector);
> if (conn_state->crtc && conn_state->crtc != crtc) {
> crtc_state = drm_atomic_get_existing_crtc_state(conn_state->state,
> conn_state->crtc);
> @@ -1177,6 +1171,8 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
> 1 << drm_connector_index(conn_state->connector);
> }
>
> + if (conn_state->crtc)
> + drm_connector_unreference(conn_state->connector);
> conn_state->crtc = crtc;
>
> if (crtc)
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index d25abce..c48446d 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2762,6 +2762,8 @@ __drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector,
> struct drm_connector_state *state)
> {
> memcpy(state, connector->state, sizeof(*state));
> + if (state->crtc)
> + drm_connector_reference(connector);
> }
> EXPORT_SYMBOL(__drm_atomic_helper_connector_duplicate_state);
>
> @@ -2889,6 +2891,8 @@ __drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
> * state will automatically do the right thing if code is ever added
> * to this function.
> */
> + if (state->crtc)
> + drm_connector_unreference(state->connector);
> }
> EXPORT_SYMBOL(__drm_atomic_helper_connector_destroy_state);
>
> --
> 2.5.5
>
> _______________________________________________
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/atomic: use connector references (v3)
2016-05-03 20:09 ` Daniel Vetter
@ 2016-05-03 20:13 ` Daniel Stone
0 siblings, 0 replies; 3+ messages in thread
From: Daniel Stone @ 2016-05-03 20:13 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel
Hi,
On 3 May 2016 at 21:09, Daniel Vetter <daniel@ffwll.ch> wrote:
> This patch series seems cursed with lots of embarrassement for reviewers.
> I totally missed/forgot the FIXME in drm_atomic_state_default_clear(),
> which this patch fixes. Anyway, I'm happy to burn my hands again ;-)
Let's share the embarrassment. What could possibly go wrong?
Reviewed-by: Daniel Stone <daniels@collabora.com>
Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-05-03 20:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-03 20:03 [PATCH] drm/atomic: use connector references (v3) Dave Airlie
2016-05-03 20:09 ` Daniel Vetter
2016-05-03 20:13 ` Daniel Stone
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.