[Intel-gfx] [PATCH] drm/i915: don't disable disconnected outputs

[Rodrigo Vivi]

makes sense
Reviewed-by: Rodrigo Vivi <rodrigo.vivi at gmail.com>

couldn't we close the bug already?

On Tue, Dec 18, 2012 at 6:37 AM, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> This piece of neat lore has been ported painstakingly and bug-for-bug
> compatible from the old crtc helper code.
>
> Imo it's utter nonsense.
>
> If you disconnected a cable and before you reconnect it, userspace (or
> the kernel) does an set_crtc call, this will result in that connector
> getting disabled. Which will result in a nice black screen when
> plugging in the cable again.
>
> There's absolutely no reason the kernel does such policy enforcements
> - if userspace tries to set up a mode on something disconnected we
> might fail loudly (since the dp link training fails), but silently
> adjusting the output configuration behind userspace's back is a recipe
> for disaster. Specifically I think that this could explain some of our
> MI_WAIT hangs around suspend, where userspace issues a scanline wait
> on a disable pipe. This mechanisims here could explain how that pipe
> got disabled without userspace noticing.
>
> Note that this fixes a NULL deref at BIOS takeover when the firmware
> sets up a disconnected output in a clone configuration with a
> connected output on the 2nd pipe: When doing the full modeset we don't
> have a mode for the 2nd pipe and OOPS. On the first pipe this doesn't
> matter, since at boot-up the fbdev helpers will set up the choosen
> configuration on that on first. Since this is now the umptenth bug
> around handling this imo brain-dead semantics correctly, I think it's
> time to kill it and see whether there's any userspace out there which
> relies on this.
>
> It also nicely demonstrates that we have a tiny window where DP
> hotplug can still kill the driver.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=58396
> Cc: stable at vger.kernel.org
> Tested-by: Peter Ujfalusi <peter.ujfalusi at gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 34832bc0..399f862 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7632,10 +7632,6 @@ intel_modeset_stage_output_state(struct drm_device *dev,
>                         DRM_DEBUG_KMS("encoder changed, full mode switch\n");
>                         config->mode_changed = true;
>                 }
> -
> -               /* Disable all disconnected encoders. */
> -               if (connector->base.status == connector_status_disconnected)
> -                       connector->new_encoder = NULL;
>         }
>         /* connector->new_encoder is now updated for all connectors. */
>
> --
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br