On Mon, Dec 07, 2015 at 11:13:20PM +0200, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx
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@xxxxxxxx>
> Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@xxxxxxxxx>
> Fixes: 5448a00d3f06 ("drm/i915: Don't use staged config in check_digital_port_conflicts()")
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html