From: Egbert Eich <eich@suse.de>
To: intel-gfx@lists.freedesktop.org
Cc: Egbert Eich <eich@suse.de>, Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: [PATCH] drm/i915: Break encoder->crtc link separately in intel_sanitize_crtc()
Date: Wed, 16 Apr 2014 11:16:45 +0200 [thread overview]
Message-ID: <1397639805-12229-1-git-send-email-eich@suse.de> (raw)
In-Reply-To: <20140415190853.GC1023@phenom.ffwll.local>
Depending on the SDVO output_flags SDVO may have multiple connectors
linking to the same encoder (in intel_connector->encoder->base).
Only one of those connectors should be active (ie link to the encoder
thru drm_connector->encoder).
If intel_connector_break_all_links() is called from intel_sanitize_crtc()
we may break the crtc connection of an encoder thru an inactive connector
in which case intel_connector_break_all_links() will not be called again
for the active connector if this happens to come later in the list due to:
if (connector->encoder->base.crtc != &crtc->base)
continue;
in intel_sanitize_crtc().
This will however leave the drm_connector->encoder linkage for this
active connector in place. Subsequently this will cause multiple
warnings in intel_connector_check_state() to trigger and the driver
will eventually die in drm_encoder_crtc_ok() (because of crtc == NULL).
To avoid this remove intel_connector_break_all_links() and move its
code to its two calling functions: intel_sanitize_crtc() and
intel_sanitize_encoder().
This allows to implement the link breaking more flexibly matching
the surrounding code: ie. in intel_sanitize_crtc() we can break the
crtc link separatly after the links to the encoders have been
broken which avoids above problem.
Signed-off-by: Egbert Eich <eich@suse.de>
v2: This patch takes care of the concernes voiced by Chris Wilson
and Daniel Vetter that only breaking links if the drm_connector
is linked to an encoder may miss some links.
---
drivers/gpu/drm/i915/intel_display.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1390ab5..c276733 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11386,15 +11386,6 @@ void intel_modeset_init(struct drm_device *dev)
}
}
-static void
-intel_connector_break_all_links(struct intel_connector *connector)
-{
- connector->base.dpms = DRM_MODE_DPMS_OFF;
- connector->base.encoder = NULL;
- connector->encoder->connectors_active = false;
- connector->encoder->base.crtc = NULL;
-}
-
static void intel_enable_pipe_a(struct drm_device *dev)
{
struct intel_connector *connector;
@@ -11476,8 +11467,16 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
if (connector->encoder->base.crtc != &crtc->base)
continue;
- intel_connector_break_all_links(connector);
+ connector->base.dpms = DRM_MODE_DPMS_OFF;
+ connector->encoder->connectors_active = false;
+ connector->base.encoder = NULL;
}
+ /* multiple connectors may have the same encoder:
+ * break crtc link separately */
+ list_for_each_entry(connector, &dev->mode_config.connector_list,
+ base.head)
+ if (connector->encoder->base.crtc == &crtc->base)
+ connector->encoder->base.crtc = NULL;
WARN_ON(crtc->active);
crtc->base.enabled = false;
@@ -11559,6 +11558,8 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder)
drm_get_encoder_name(&encoder->base));
encoder->disable(encoder);
}
+ encoder->base.crtc = NULL;
+ encoder->connectors_active = false;
/* Inconsistent output/port/pipe state happens presumably due to
* a bug in one of the get_hw_state functions. Or someplace else
@@ -11569,8 +11570,8 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder)
base.head) {
if (connector->encoder != encoder)
continue;
-
- intel_connector_break_all_links(connector);
+ connector->base.dpms = DRM_MODE_DPMS_OFF;
+ connector->base.encoder = NULL;
}
}
/* Enabled encoders without active connectors will be fixed in
--
1.8.4.5
next prev parent reply other threads:[~2014-04-16 9:16 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-14 17:26 [PATCH 0/2] Fix if multiple SDVO outputs are flagged Egbert Eich
2014-04-14 17:26 ` [PATCH 1/2] drm/i915: Only break encoder linked when linked to connector Egbert Eich
2014-04-15 7:46 ` Chris Wilson
2014-04-15 9:14 ` Egbert Eich
2014-04-15 19:08 ` Daniel Vetter
2014-04-16 6:04 ` Egbert Eich
2014-04-16 9:16 ` Egbert Eich [this message]
2014-04-16 11:42 ` [PATCH] drm/i915: Break encoder->crtc link separately in intel_sanitize_crtc() Daniel Vetter
2014-04-25 8:56 ` [PATCH v3] " Egbert Eich
2014-04-25 11:28 ` Jani Nikula
2014-04-14 17:26 ` [PATCH 2/2] drm/i915: Set up SDVO encoder type only at detect Egbert Eich
2014-04-15 9:39 ` Chris Wilson
2014-04-15 19:14 ` [PATCH v2] " Egbert Eich
2014-04-15 19:19 ` Chris Wilson
2014-04-15 19:12 ` [PATCH 2/2] " Daniel Vetter
2014-04-16 5:58 ` Egbert Eich
2014-04-16 7:55 ` Daniel Vetter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1397639805-12229-1-git-send-email-eich@suse.de \
--to=eich@suse.de \
--cc=daniel.vetter@ffwll.ch \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.