* [PATCH] drm/i915: Correctly refcount connectors in hw state readou
@ 2016-05-06 13:39 Daniel Vetter
2016-05-06 14:01 ` Chris Wilson
2016-05-06 14:07 ` Ville Syrjälä
0 siblings, 2 replies; 7+ messages in thread
From: Daniel Vetter @ 2016-05-06 13:39 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development
Cc: Dave Airlie, Daniel Vetter, Daniel Vetter
This was forgotten when adding the the refcounting to
drm_connector_state.
v2: Don't forget to unreference existing connectors. This isn't
relevant on driver load, but this code also runs on resume, and there
we already have an atomic state. Spotted by Chris Wilson.
Cc: Gabriel Feceoru <gabriel.feceoru@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Marius Vlad <marius.c.vlad@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Fixes: d2307dea14a4 ("drm/atomic: use connector references (v3)")
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 45c218db86be..6ba676f81bae 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12023,11 +12023,16 @@ static void intel_modeset_update_connector_atomic_state(struct drm_device *dev)
struct intel_connector *connector;
for_each_intel_connector(dev, connector) {
+ if (connector->base.state->crtc)
+ drm_connector_unreference(&connector->base);
+
if (connector->base.encoder) {
connector->base.state->best_encoder =
connector->base.encoder;
connector->base.state->crtc =
connector->base.encoder->crtc;
+
+ drm_connector_reference(&connector->base);
} else {
connector->base.state->best_encoder = NULL;
connector->base.state->crtc = NULL;
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] drm/i915: Correctly refcount connectors in hw state readou
2016-05-06 13:39 [PATCH] drm/i915: Correctly refcount connectors in hw state readou Daniel Vetter
@ 2016-05-06 14:01 ` Chris Wilson
2016-05-06 14:09 ` Daniel Vetter
2016-05-06 14:07 ` Ville Syrjälä
1 sibling, 1 reply; 7+ messages in thread
From: Chris Wilson @ 2016-05-06 14:01 UTC (permalink / raw)
To: Daniel Vetter
Cc: Gabriel Feceoru, Intel Graphics Development, DRI Development,
Dave Airlie, Daniel Vetter
On Fri, May 06, 2016 at 03:39:03PM +0200, Daniel Vetter wrote:
> This was forgotten when adding the the refcounting to
> drm_connector_state.
>
> v2: Don't forget to unreference existing connectors. This isn't
> relevant on driver load, but this code also runs on resume, and there
> we already have an atomic state. Spotted by Chris Wilson.
>
> Cc: Gabriel Feceoru <gabriel.feceoru@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Marius Vlad <marius.c.vlad@intel.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Fixes: d2307dea14a4 ("drm/atomic: use connector references (v3)")
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] drm/i915: Correctly refcount connectors in hw state readou
2016-05-06 14:01 ` Chris Wilson
@ 2016-05-06 14:09 ` Daniel Vetter
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2016-05-06 14:09 UTC (permalink / raw)
To: Chris Wilson, Daniel Vetter, Intel Graphics Development,
DRI Development, Gabriel Feceoru, Marius Vlad, Dave Airlie,
Daniel Vetter
On Fri, May 06, 2016 at 03:01:17PM +0100, Chris Wilson wrote:
> On Fri, May 06, 2016 at 03:39:03PM +0200, Daniel Vetter wrote:
> > This was forgotten when adding the the refcounting to
> > drm_connector_state.
> >
> > v2: Don't forget to unreference existing connectors. This isn't
> > relevant on driver load, but this code also runs on resume, and there
> > we already have an atomic state. Spotted by Chris Wilson.
> >
> > Cc: Gabriel Feceoru <gabriel.feceoru@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Marius Vlad <marius.c.vlad@intel.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Fixes: d2307dea14a4 ("drm/atomic: use connector references (v3)")
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Thanks for the review, applied to drm-misc.
-Daniel
--
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] 7+ messages in thread
* Re: [PATCH] drm/i915: Correctly refcount connectors in hw state readou
2016-05-06 13:39 [PATCH] drm/i915: Correctly refcount connectors in hw state readou Daniel Vetter
2016-05-06 14:01 ` Chris Wilson
@ 2016-05-06 14:07 ` Ville Syrjälä
2016-05-06 14:19 ` Chris Wilson
1 sibling, 1 reply; 7+ messages in thread
From: Ville Syrjälä @ 2016-05-06 14:07 UTC (permalink / raw)
To: Daniel Vetter
Cc: Daniel Vetter, Dave Airlie, Intel Graphics Development,
DRI Development, Gabriel Feceoru
On Fri, May 06, 2016 at 03:39:03PM +0200, Daniel Vetter wrote:
> This was forgotten when adding the the refcounting to
> drm_connector_state.
>
> v2: Don't forget to unreference existing connectors. This isn't
> relevant on driver load, but this code also runs on resume, and there
> we already have an atomic state. Spotted by Chris Wilson.
>
> Cc: Gabriel Feceoru <gabriel.feceoru@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Marius Vlad <marius.c.vlad@intel.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Fixes: d2307dea14a4 ("drm/atomic: use connector references (v3)")
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 45c218db86be..6ba676f81bae 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12023,11 +12023,16 @@ static void intel_modeset_update_connector_atomic_state(struct drm_device *dev)
> struct intel_connector *connector;
>
> for_each_intel_connector(dev, connector) {
> + if (connector->base.state->crtc)
> + drm_connector_unreference(&connector->base);
> +
> if (connector->base.encoder) {
> connector->base.state->best_encoder =
> connector->base.encoder;
> connector->base.state->crtc =
> connector->base.encoder->crtc;
> +
> + drm_connector_reference(&connector->base);
Is it safe to unref before ref?
> } else {
> connector->base.state->best_encoder = NULL;
> connector->base.state->crtc = NULL;
> --
> 2.5.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
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] 7+ messages in thread* Re: [PATCH] drm/i915: Correctly refcount connectors in hw state readou
2016-05-06 14:07 ` Ville Syrjälä
@ 2016-05-06 14:19 ` Chris Wilson
0 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2016-05-06 14:19 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Daniel Vetter, Intel Graphics Development, Dave Airlie,
DRI Development, Daniel Vetter
On Fri, May 06, 2016 at 05:07:09PM +0300, Ville Syrjälä wrote:
> On Fri, May 06, 2016 at 03:39:03PM +0200, Daniel Vetter wrote:
> > This was forgotten when adding the the refcounting to
> > drm_connector_state.
> >
> > v2: Don't forget to unreference existing connectors. This isn't
> > relevant on driver load, but this code also runs on resume, and there
> > we already have an atomic state. Spotted by Chris Wilson.
> >
> > Cc: Gabriel Feceoru <gabriel.feceoru@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Marius Vlad <marius.c.vlad@intel.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Fixes: d2307dea14a4 ("drm/atomic: use connector references (v3)")
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 45c218db86be..6ba676f81bae 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -12023,11 +12023,16 @@ static void intel_modeset_update_connector_atomic_state(struct drm_device *dev)
> > struct intel_connector *connector;
> >
> > for_each_intel_connector(dev, connector) {
> > + if (connector->base.state->crtc)
> > + drm_connector_unreference(&connector->base);
> > +
> > if (connector->base.encoder) {
> > connector->base.state->best_encoder =
> > connector->base.encoder;
> > connector->base.state->crtc =
> > connector->base.encoder->crtc;
> > +
> > + drm_connector_reference(&connector->base);
>
> Is it safe to unref before ref?
In this case, I believe we can assert that we hold a reference to the
connector (as part of the driver state) and the atomic state holds a
reference, i.e. the reference count here should be at least 2. If it was
freed, we would get a warn from kref as well as memdebug (if enabled).
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] drm/i915: Correctly refcount connectors in hw state readou
@ 2016-05-06 13:09 Daniel Vetter
2016-05-06 13:20 ` Chris Wilson
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Vetter @ 2016-05-06 13:09 UTC (permalink / raw)
To: DRI Development
Cc: Daniel Vetter, Intel Graphics Development, Gabriel Feceoru,
Dave Airlie
This was forgotten when adding the the refcounting to
drm_connector_state.
Cc: Gabriel Feceoru <gabriel.feceoru@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Marius Vlad <marius.c.vlad@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Fixes: d2307dea14a4 ("drm/atomic: use connector references (v3)")
---
drivers/gpu/drm/i915/intel_display.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 45c218db86be..d26429c77c10 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12028,6 +12028,8 @@ static void intel_modeset_update_connector_atomic_state(struct drm_device *dev)
connector->base.encoder;
connector->base.state->crtc =
connector->base.encoder->crtc;
+
+ drm_connector_reference(&connector->base);
} else {
connector->base.state->best_encoder = NULL;
connector->base.state->crtc = NULL;
--
2.5.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] drm/i915: Correctly refcount connectors in hw state readou
2016-05-06 13:09 Daniel Vetter
@ 2016-05-06 13:20 ` Chris Wilson
0 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2016-05-06 13:20 UTC (permalink / raw)
To: Daniel Vetter
Cc: Gabriel Feceoru, Intel Graphics Development, Dave Airlie,
DRI Development
On Fri, May 06, 2016 at 03:09:43PM +0200, Daniel Vetter wrote:
> This was forgotten when adding the the refcounting to
> drm_connector_state.
>
> Cc: Gabriel Feceoru <gabriel.feceoru@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Marius Vlad <marius.c.vlad@intel.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Fixes: d2307dea14a4 ("drm/atomic: use connector references (v3)")
> ---
> drivers/gpu/drm/i915/intel_display.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 45c218db86be..d26429c77c10 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12028,6 +12028,8 @@ static void intel_modeset_update_connector_atomic_state(struct drm_device *dev)
> connector->base.encoder;
> connector->base.state->crtc =
> connector->base.encoder->crtc;
> +
> + drm_connector_reference(&connector->base);
Right that matches d2307dea14a4 but do we have to worry about balance
here?
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8ae5a8ac3b7b..49c4f5e77d32 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11947,14 +11947,18 @@ static void intel_modeset_update_connector_atomic_state(struct drm_device *dev)
struct intel_connector *connector;
for_each_intel_connector(dev, connector) {
+ if (connector->base.state->crtc) {
+ connector->base.state->best_encoder = NULL;
+ connector->base.state->crtc = NULL;
+ drm_connector_unreference(&connector->base);
+ }
+
if (connector->base.encoder) {
- connector->base.state->best_encoder =
- connector->base.encoder;
+ drm_connector_reference(&connector->base);
connector->base.state->crtc =
connector->base.encoder->crtc;
- } else {
- connector->base.state->best_encoder = NULL;
- connector->base.state->crtc = NULL;
+ connector->base.state->best_encoder =
+ connector->base.encoder;
}
}
}
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-05-06 14:19 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-06 13:39 [PATCH] drm/i915: Correctly refcount connectors in hw state readou Daniel Vetter
2016-05-06 14:01 ` Chris Wilson
2016-05-06 14:09 ` Daniel Vetter
2016-05-06 14:07 ` Ville Syrjälä
2016-05-06 14:19 ` Chris Wilson
-- strict thread matches above, loose matches on Subject: below --
2016-05-06 13:09 Daniel Vetter
2016-05-06 13:20 ` Chris Wilson
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.