All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ander Conselvan de Oliveira
	<ander.conselvan.de.oliveira@intel.com>,
	intel-gfx@lists.freedesktop.org, stable@vger.kernel.org
Subject: Re: [PATCH] drm/i915: Unbreak check_digital_port_conflicts()
Date: Thu, 10 Dec 2015 15:19:24 +0200	[thread overview]
Message-ID: <20151210131924.GP4437@intel.com> (raw)
In-Reply-To: <877fkmlegs.fsf@intel.com>

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Daniel Vetter <daniel@ffwll.ch>,
	Ander Conselvan de Oliveira
	<ander.conselvan.de.oliveira@intel.com>,
	intel-gfx@lists.freedesktop.org, stable@vger.kernel.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Unbreak check_digital_port_conflicts()
Date: Thu, 10 Dec 2015 15:19:24 +0200	[thread overview]
Message-ID: <20151210131924.GP4437@intel.com> (raw)
In-Reply-To: <877fkmlegs.fsf@intel.com>

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

  reply	other threads:[~2015-12-10 13:19 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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  9:52   ` [Intel-gfx] " Daniel Vetter
2015-12-10 11:39   ` Jani Nikula
2015-12-10 11:39     ` [Intel-gfx] " Jani Nikula
2015-12-10 13:19     ` Ville Syrjälä [this message]
2015-12-10 13:19       ` Ville Syrjälä
2015-12-10 16:22 ` [PATCH v2] " ville.syrjala
2015-12-22 12:29   ` Jani Nikula

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=20151210131924.GP4437@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=ander.conselvan.de.oliveira@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=stable@vger.kernel.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.