* [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).