* [PATCH] drm/i915: Unbreak check_digital_port_conflicts()
@ 2015-12-07 21:13 ville.syrjala
2015-12-10 9:52 ` [Intel-gfx] " Daniel Vetter
2015-12-10 16:22 ` [PATCH v2] " ville.syrjala
0 siblings, 2 replies; 6+ messages in thread
From: ville.syrjala @ 2015-12-07 21:13 UTC (permalink / raw)
To: intel-gfx; +Cc: stable, Ander Conselvan de Oliveira
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Atomic changes broke check_digital_port_conflicts(). It needs to look
at the global situation instead of just trying to find a conflict
within the current atomic state.
This bug made my HSW explode spectacularly after I had split the DDI
encoders into separate DP and HDMI encoders. With the fix, things
seem much more solid.
I hope holding the connection_mutex is enough protection that we can
actually walk the connectors even if they're not part of the current
atomic state...
Cc: stable@vger.kernel.org
Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Fixes: 5448a00d3f06 ("drm/i915: Don't use staged config in check_digital_port_conflicts()")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1c6d56c84b9d..c902964ceca0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12253,18 +12253,23 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
static bool check_digital_port_conflicts(struct drm_atomic_state *state)
{
- struct intel_encoder *encoder;
+ struct drm_device *dev = state->dev;
struct drm_connector *connector;
- struct drm_connector_state *connector_state;
unsigned int used_ports = 0;
- int i;
/*
* Walk the connector list instead of the encoder
* list to detect the problem on ddi platforms
* where there's just one encoder per digital port.
*/
- for_each_connector_in_state(state, connector, connector_state, i) {
+ drm_for_each_connector(connector, dev) {
+ struct drm_connector_state *connector_state;
+ struct intel_encoder *encoder;
+
+ connector_state = drm_atomic_get_existing_connector_state(state, connector);
+ if (!connector_state)
+ connector_state = connector->state;
+
if (!connector_state->best_encoder)
continue;
--
2.4.10
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Unbreak check_digital_port_conflicts() 2015-12-07 21:13 [PATCH] drm/i915: Unbreak check_digital_port_conflicts() ville.syrjala @ 2015-12-10 9:52 ` Daniel Vetter 2015-12-10 11:39 ` Jani Nikula 2015-12-10 16:22 ` [PATCH v2] " ville.syrjala 1 sibling, 1 reply; 6+ messages in thread From: Daniel Vetter @ 2015-12-10 9:52 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx, Ander Conselvan de Oliveira, stable On Mon, Dec 07, 2015 at 11:13:20PM +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > Atomic changes broke check_digital_port_conflicts(). It needs to look > at the global situation instead of just trying to find a conflict > within the current atomic state. > > This bug made my HSW explode spectacularly after I had split the DDI > encoders into separate DP and HDMI encoders. With the fix, things > seem much more solid. > > I hope holding the connection_mutex is enough protection that we can > actually walk the connectors even if they're not part of the current > atomic state... That is sufficient locking. > > Cc: stable@vger.kernel.org Ugh. Long-term I think what we need is for all drivers (well at least atomic ones) to fill out the possible_clones stuff correctly in the encoder. And then check this in the atomic helpers. But that's way too much for -fixes. On the patch itself, for -fixes: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > Fixes: 5448a00d3f06 ("drm/i915: Don't use staged config in check_digital_port_conflicts()") > Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 1c6d56c84b9d..c902964ceca0 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12253,18 +12253,23 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc, > > static bool check_digital_port_conflicts(struct drm_atomic_state *state) > { > - struct intel_encoder *encoder; > + struct drm_device *dev = state->dev; > struct drm_connector *connector; > - struct drm_connector_state *connector_state; > unsigned int used_ports = 0; > - int i; > > /* > * Walk the connector list instead of the encoder > * list to detect the problem on ddi platforms > * where there's just one encoder per digital port. > */ > - for_each_connector_in_state(state, connector, connector_state, i) { > + drm_for_each_connector(connector, dev) { > + struct drm_connector_state *connector_state; > + struct intel_encoder *encoder; > + > + connector_state = drm_atomic_get_existing_connector_state(state, connector); > + if (!connector_state) > + connector_state = connector->state; > + > if (!connector_state->best_encoder) > continue; > > -- > 2.4.10 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Unbreak check_digital_port_conflicts() 2015-12-10 9:52 ` [Intel-gfx] " Daniel Vetter @ 2015-12-10 11:39 ` Jani Nikula 2015-12-10 13:19 ` Ville Syrjälä 0 siblings, 1 reply; 6+ messages in thread From: Jani Nikula @ 2015-12-10 11:39 UTC (permalink / raw) To: Daniel Vetter, ville.syrjala Cc: Ander Conselvan de Oliveira, intel-gfx, stable On Thu, 10 Dec 2015, Daniel Vetter <daniel@ffwll.ch> wrote: > On Mon, Dec 07, 2015 at 11:13:20PM +0200, ville.syrjala@linux.intel.com wrote: >> From: Ville Syrjälä <ville.syrjala@linux.intel.com> >> >> Atomic changes broke check_digital_port_conflicts(). It needs to look >> at the global situation instead of just trying to find a conflict >> within the current atomic state. >> >> This bug made my HSW explode spectacularly after I had split the DDI >> encoders into separate DP and HDMI encoders. With the fix, things >> seem much more solid. >> >> I hope holding the connection_mutex is enough protection that we can >> actually walk the connectors even if they're not part of the current >> atomic state... > > That is sufficient locking. >> >> Cc: stable@vger.kernel.org > > Ugh. Long-term I think what we need is for all drivers (well at least > atomic ones) to fill out the possible_clones stuff correctly in the > encoder. And then check this in the atomic helpers. But that's way too > much for -fixes. > > On the patch itself, for -fixes: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Fails to apply in either fixes or dinq. Ville, please update. BR, Jani. > >> Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> >> Fixes: 5448a00d3f06 ("drm/i915: Don't use staged config in check_digital_port_conflicts()") >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_display.c | 13 +++++++++---- >> 1 file changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 1c6d56c84b9d..c902964ceca0 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -12253,18 +12253,23 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc, >> >> static bool check_digital_port_conflicts(struct drm_atomic_state *state) >> { >> - struct intel_encoder *encoder; >> + struct drm_device *dev = state->dev; >> struct drm_connector *connector; >> - struct drm_connector_state *connector_state; >> unsigned int used_ports = 0; >> - int i; >> >> /* >> * Walk the connector list instead of the encoder >> * list to detect the problem on ddi platforms >> * where there's just one encoder per digital port. >> */ >> - for_each_connector_in_state(state, connector, connector_state, i) { >> + drm_for_each_connector(connector, dev) { >> + struct drm_connector_state *connector_state; >> + struct intel_encoder *encoder; >> + >> + connector_state = drm_atomic_get_existing_connector_state(state, connector); >> + if (!connector_state) >> + connector_state = connector->state; >> + >> if (!connector_state->best_encoder) >> continue; >> >> -- >> 2.4.10 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Unbreak check_digital_port_conflicts() 2015-12-10 11:39 ` Jani Nikula @ 2015-12-10 13:19 ` Ville Syrjälä 0 siblings, 0 replies; 6+ messages in thread From: Ville Syrjälä @ 2015-12-10 13:19 UTC (permalink / raw) To: Jani Nikula; +Cc: Daniel Vetter, Ander Conselvan de Oliveira, intel-gfx, stable On Thu, Dec 10, 2015 at 01:39:47PM +0200, Jani Nikula wrote: > On Thu, 10 Dec 2015, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Mon, Dec 07, 2015 at 11:13:20PM +0200, ville.syrjala@linux.intel.com wrote: > >> From: Ville Syrj�l� <ville.syrjala@linux.intel.com> > >> > >> Atomic changes broke check_digital_port_conflicts(). It needs to look > >> at the global situation instead of just trying to find a conflict > >> within the current atomic state. > >> > >> This bug made my HSW explode spectacularly after I had split the DDI > >> encoders into separate DP and HDMI encoders. With the fix, things > >> seem much more solid. > >> > >> I hope holding the connection_mutex is enough protection that we can > >> actually walk the connectors even if they're not part of the current > >> atomic state... > > > > That is sufficient locking. > >> > >> Cc: stable@vger.kernel.org > > > > Ugh. Long-term I think what we need is for all drivers (well at least > > atomic ones) to fill out the possible_clones stuff correctly in the > > encoder. And then check this in the atomic helpers. But that's way too > > much for -fixes. > > > > On the patch itself, for -fixes: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Fails to apply in either fixes or dinq. Ville, please update. Ugh. I guess it was sitting on top of something else when I git format-patched it. Sorry about that, will resend. > > BR, > Jani. > > > > > >> Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > >> Fixes: 5448a00d3f06 ("drm/i915: Don't use staged config in check_digital_port_conflicts()") > >> Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com> > >> --- > >> drivers/gpu/drm/i915/intel_display.c | 13 +++++++++---- > >> 1 file changed, 9 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > >> index 1c6d56c84b9d..c902964ceca0 100644 > >> --- a/drivers/gpu/drm/i915/intel_display.c > >> +++ b/drivers/gpu/drm/i915/intel_display.c > >> @@ -12253,18 +12253,23 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc, > >> > >> static bool check_digital_port_conflicts(struct drm_atomic_state *state) > >> { > >> - struct intel_encoder *encoder; > >> + struct drm_device *dev = state->dev; > >> struct drm_connector *connector; > >> - struct drm_connector_state *connector_state; > >> unsigned int used_ports = 0; > >> - int i; > >> > >> /* > >> * Walk the connector list instead of the encoder > >> * list to detect the problem on ddi platforms > >> * where there's just one encoder per digital port. > >> */ > >> - for_each_connector_in_state(state, connector, connector_state, i) { > >> + drm_for_each_connector(connector, dev) { > >> + struct drm_connector_state *connector_state; > >> + struct intel_encoder *encoder; > >> + > >> + connector_state = drm_atomic_get_existing_connector_state(state, connector); > >> + if (!connector_state) > >> + connector_state = connector->state; > >> + > >> if (!connector_state->best_encoder) > >> continue; > >> > >> -- > >> 2.4.10 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Jani Nikula, Intel Open Source Technology Center -- Ville Syrj�l� Intel OTC ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] drm/i915: Unbreak check_digital_port_conflicts() 2015-12-07 21:13 [PATCH] drm/i915: Unbreak check_digital_port_conflicts() ville.syrjala 2015-12-10 9:52 ` [Intel-gfx] " Daniel Vetter @ 2015-12-10 16:22 ` ville.syrjala 2015-12-22 12:29 ` Jani Nikula 1 sibling, 1 reply; 6+ messages in thread From: ville.syrjala @ 2015-12-10 16:22 UTC (permalink / raw) To: intel-gfx; +Cc: Jani Nikula, stable, Ander Conselvan de Oliveira From: Ville Syrjälä <ville.syrjala@linux.intel.com> Atomic changes broke check_digital_port_conflicts(). It needs to look at the global situation instead of just trying to find a conflict within the current atomic state. This bug made my HSW explode spectacularly after I had split the DDI encoders into separate DP and HDMI encoders. With the fix, things seem much more solid. I hope holding the connection_mutex is enough protection that we can actually walk the connectors even if they're not part of the current atomic state... v2: Regenerate the patch so that it actually applies (Jani) Cc: stable@vger.kernel.org Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Fixes: 5448a00d3f06 ("drm/i915: Don't use staged config in check_digital_port_conflicts()") Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/intel_display.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 930661de11ff..10974424bdfa 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12260,18 +12260,22 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc, static bool check_digital_port_conflicts(struct drm_atomic_state *state) { struct drm_device *dev = state->dev; - struct intel_encoder *encoder; struct drm_connector *connector; - struct drm_connector_state *connector_state; unsigned int used_ports = 0; - int i; /* * Walk the connector list instead of the encoder * list to detect the problem on ddi platforms * where there's just one encoder per digital port. */ - for_each_connector_in_state(state, connector, connector_state, i) { + drm_for_each_connector(connector, dev) { + struct drm_connector_state *connector_state; + struct intel_encoder *encoder; + + connector_state = drm_atomic_get_existing_connector_state(state, connector); + if (!connector_state) + connector_state = connector->state; + if (!connector_state->best_encoder) continue; -- 2.4.10 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/i915: Unbreak check_digital_port_conflicts() 2015-12-10 16:22 ` [PATCH v2] " ville.syrjala @ 2015-12-22 12:29 ` Jani Nikula 0 siblings, 0 replies; 6+ messages in thread From: Jani Nikula @ 2015-12-22 12:29 UTC (permalink / raw) To: ville.syrjala, intel-gfx; +Cc: stable, Ander Conselvan de Oliveira On Thu, 10 Dec 2015, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Atomic changes broke check_digital_port_conflicts(). It needs to look > at the global situation instead of just trying to find a conflict > within the current atomic state. > > This bug made my HSW explode spectacularly after I had split the DDI > encoders into separate DP and HDMI encoders. With the fix, things > seem much more solid. > > I hope holding the connection_mutex is enough protection that we can > actually walk the connectors even if they're not part of the current > atomic state... > > v2: Regenerate the patch so that it actually applies (Jani) > > Cc: stable@vger.kernel.org > Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > Fixes: 5448a00d3f06 ("drm/i915: Don't use staged config in check_digital_port_conflicts()") > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Pushed to drm-intel-next-queued, thanks for the patch and review. BR, Jani. > --- > drivers/gpu/drm/i915/intel_display.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 930661de11ff..10974424bdfa 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12260,18 +12260,22 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc, > static bool check_digital_port_conflicts(struct drm_atomic_state *state) > { > struct drm_device *dev = state->dev; > - struct intel_encoder *encoder; > struct drm_connector *connector; > - struct drm_connector_state *connector_state; > unsigned int used_ports = 0; > - int i; > > /* > * Walk the connector list instead of the encoder > * list to detect the problem on ddi platforms > * where there's just one encoder per digital port. > */ > - for_each_connector_in_state(state, connector, connector_state, i) { > + drm_for_each_connector(connector, dev) { > + struct drm_connector_state *connector_state; > + struct intel_encoder *encoder; > + > + connector_state = drm_atomic_get_existing_connector_state(state, connector); > + if (!connector_state) > + connector_state = connector->state; > + > if (!connector_state->best_encoder) > continue; -- Jani Nikula, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-12-22 12:29 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-12-07 21:13 [PATCH] drm/i915: Unbreak check_digital_port_conflicts() ville.syrjala 2015-12-10 9:52 ` [Intel-gfx] " Daniel Vetter 2015-12-10 11:39 ` Jani Nikula 2015-12-10 13:19 ` Ville Syrjälä 2015-12-10 16:22 ` [PATCH v2] " ville.syrjala 2015-12-22 12:29 ` Jani Nikula
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).