On Thu, Dec 10, 2015 at 05:57:57PM -0800, Matt Roper wrote:
> On Thu, Dec 10, 2015 at 03:14:38PM +0100, Daniel Vetter wrote:
> > On Tue, Dec 08, 2015 at 02:48:51PM -0800, Matt Roper wrote:
> > > In all of our various SDVO setup functions, we allocate an SDVO
> > > connector (along with an associated connector->state) object, then
> > > perform initialization. If that initialization fails, we need to make
> > > sure to free the state object as well as the connector.
> > >
> > > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> >
> > Where do we alloc connector->state in sdvo_connector_init()? I thought the
> > flow is 1) register all the kms objects with our pile of _init() functions
> > 2) do hw readout, which is the thing which creates all these states.
>
> It's allocated by intel_sdvo_connector_alloc -> intel_connector_init
> which gets run before intel_sdvo_connector_init() even gets called.
> There are two separate "init" functions that get called for SDVO
> connectors...the first allocates the state object, but the second can
> fail too and trigger cleanup (which was failing to destroy the state).
Ah right, didn't find that. Started a full audit but stuck at
intel_dp_init. That has a kfree(intel_connector), but seems to miss this
too.
On the patch itself: Can't we just call drm_connector_cleanup? And an
intel_connectore_free which wraps that + the kfree would be nice. Well,
that would mean we'd need to move drm_connector_init into
intel_connector_init, but I think that would make a lot of sense either
way.
-Daniel
>
> >
> > None of the other connectors seem to have this issue (or at least I don't
> > see a patch for them).
>
> I don't think any of the other connector types have a second, special
> init function that gets run after intel_connector_init() is finished
> that I can see.
>
>
> Matt
>
> > -Daniel
> >
> > > ---
> > > drivers/gpu/drm/i915/intel_sdvo.c | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> > > index 06679f1..ff28867 100644
> > > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > > @@ -2479,6 +2479,8 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
> > > }
> > >
> > > if (intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo) < 0) {
> > > + drm_atomic_helper_connector_destroy_state(connector,
> > > + connector->state);
> > > kfree(intel_sdvo_connector);
> > > return false;
> > > }
> > > @@ -2514,6 +2516,8 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int type)
> > > intel_sdvo->is_tv = true;
> > >
> > > if (intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo) < 0) {
> > > + drm_atomic_helper_connector_destroy_state(connector,
> > > + connector->state);
> > > kfree(intel_sdvo_connector);
> > > return false;
> > > }
> > > @@ -2561,6 +2565,8 @@ intel_sdvo_analog_init(struct intel_sdvo *intel_sdvo, int device)
> > > }
> > >
> > > if (intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo) < 0) {
> > > + drm_atomic_helper_connector_destroy_state(connector,
> > > + connector->state);
> > > kfree(intel_sdvo_connector);
> > > return false;
> > > }
> > > @@ -2596,6 +2602,8 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
> > > }
> > >
> > > if (intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo) < 0) {
> > > + drm_atomic_helper_connector_destroy_state(connector,
> > > + connector->state);
> > > kfree(intel_sdvo_connector);
> > > return false;
> > > }
> > > --
> > > 2.1.4
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
>
> --
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
