Bug 63031

Created attachment 77323 [details]
dmesg output of non-working case.

Created attachment 77324 [details]
dmesg output of working case.

Created attachment 77328 [details] [review]
check sdvo port in sdvo connector get_hw_state

I've thought I've written this particular patch a few times by now, but it seems to be lost :( Please test.

Ah, found the patch+bug again: https://bugs.freedesktop.org/show_bug.cgi?id=60138

The patch made the settings created by intel_modeset_setup_hw_state() more consistent, such that the noise from sanitize_encoder() is now gone. 
Unfortunately, the display still does not light up. I believe the reason for this is:

[   67.744037] [drm:intel_sdvo_debug_write], SDVOB: W: 0B                         (SDVO_CMD_GET_ATTACHED_DISPLAYS)
[   67.745586] [drm:intel_sdvo_read_response], SDVOB: R: (Pending)... failed
[   67.928935] [drm:output_poll_execute], [CONNECTOR:63:VGA-2] status updated from 3 to 3

Those lines were in the attached dmesg output of the non-working case and they are still there.

You can play around with the delays in intel_sdvo_read_response() to see how long it takes for the VGA device to respond.

Created attachment 77330 [details]
log with patch from https://bugs.freedesktop.org/show_bug.cgi?id=60138 applied.

Patch from https://bugs.freedesktop.org/show_bug.cgi?id=60138 gets rid of the warnings.
Same is true for patch from attachment #77328 [details] [review].

Tested-By: Egbert Eich <eich_at_freedesktop.org>

for either one.

(In reply to comment #6)
> You can play around with the delays in intel_sdvo_read_response() to see how
> long it takes for the VGA device to respond.

Chris, I did already. I was not able to find a value where the VGA device did respond.
Currently I'm comparing the code paths taken for the case where bit 1 in GEN3_SDVOB is set when the driver is loaded (ie where a an active pipe for this encoder is found) and where it isn't as in the former case we always get a valid response.

Ok, got a bit further, 
looks like the state of the SDVOB is not consistent:
I do get valid output from SDVO_CMD_GET_ATTACHED_DISPLAYS when SDVO_CMD_SET_ACTIVE_OUTPUTS is set to 0 prior to reading SDVO_CMD_GET_ATTACHED_DISPLAYS.

To get valid output with SDVO_CMD_SET_ACTIVE_OUTPUTS set to anything but 0 some other conditions need to be met also. I have not yet been able to determine what these are.

In any case, calling encoder->disable() encoder somewhere in sanitize_encoder() for DAC-11 will get sdvo_detect() to detect the attached display correctly.

Ooops, there seems to be a bug in my santize_encoder function. Does the below snippet help?

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8e6f906..9a525e5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9074,7 +9074,7 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder)
                /* Connector is active, but has no active pipe. This is
                 * fallout from our resume register restoring. Disable
                 * the encoder manually again. */
-               if (encoder->base.crtc) {
+               if (!encoder->base.crtc) {
                        DRM_DEBUG_KMS("[ENCODER:%d:%s] manually disabled\n",
                                      encoder->base.base.id,
                                      drm_get_encoder_name(&encoder->base));

Actually this is wrong, if the encoder is active it should _always_ have a crtc. Can you please sprinkle printks and try to figure out why the encoder is active, but doesn't seem to have a crtc attached?

(In reply to comment #11)
> Actually this is wrong, if the encoder is active it should _always_ have a
> crtc. Can you please sprinkle printks and try to figure out why the encoder
> is active, but doesn't seem to have a crtc attached?


I had added such printks already. In the logs I've posted you will find lines containing "DAC-11->base.crtc <=".
In fact the encoder is not active: intel_sdvo_get_hw_state() returns false. Thus it doesn't have a crtc:
[    3.703241] [drm:intel_modeset_setup_hw_state], 2: DAC-11->base.crtc <=           (null)
Still it has active outputs: intel_sdvo_get_active_outputs() returns 2.
What I seem to have found out now:
When an encoder is disabled (bit 31 of GEN3_SDVOx is 0) then it must not have active outputs or SDVO_CMD_GET_ATTACHED_DISPLAYS will not return a result (it will remain in pending state).

Hm, maybe we should adjust the sdvo hw state code then to return true when either the SDVO port is on or when the external sdvo encoder reports an active output? Then the encoder sanitize functions would call ->disable when we want it to do so.

Created attachment 77425 [details] [review]
Fix

Daniel, indeed, your suggestion did the job.
The inconsistency in state (ie encoder active while connector inactive) gets fixed later on in intel_sanitize_crtc().

My quick patch review on irc, in case it gets lost:

danvet> egbert, inverted logic in your patch description
<danvet> it's only considered _in_active when both tests fail
<danvet> egbert, does that patch cause any issue wrt state checking?
<danvet> since right now the connector will say that it's not active, but the encoder says it is
<danvet> so I think we need to also revert my earlier patch again

Egbert, can you pls spin a v2 and then submit it to intel-gfx?

Created attachment 77442 [details] [review]
Fix backing out commit derived from attachment #77328 [details] [review].

Patch merged

commit 1e303c6febfdc23258d6ecea79c9592389a28964
Author: Egbert Eich <eich@suse.de>
Date:   Thu Apr 4 16:04:02 2013 -0400

    drm/i915: Fix SDVO connector and encoder get_hw_state functions

Ebgert, thanks a lot for going through this ordeal!