[git pull] i915 fixes

[git pull] i915 fixes

[Chris Wilson]

Hi Linus,

Dave asked to me send this pull request directly to you as he is on
holiday over the New Year. In this batch, there are a few regression
fixes. The most notable being the revert of the SSC frequency switch,
along with a use-before-initialisation introduced much earlier in the
sdvo/hdmi audio enabling and a fix for the loss of correct modesetting on
one particular DVO chipset. Eric also found another instance where we were
not adhering to the specs when setting up Ironlake. The impact is not
conclusively known but we did find a few scenarios where it delayed a
hang.
-Chris

The following changes since commit 387c31c7e5c9805b0aef8833d1731a5fe7bdea14:

  Linux 2.6.37-rc8 (2010-12-28 17:05:48 -0800)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/ickle/drm-intel.git drm-intel-fixes

Chris Wilson (4):
      drm/i915/sdvo: Add hdmi connector properties after initing the connector
      drm/i915: Verify Ironlake eDP presence on DP_A using the capability fuse
      Revert "drm/i915/bios: Reverse order of 100/120 Mhz SSC clocks"
      drm/i915/dvo: Report LVDS attached to ch701x as connected

Eric Anholt (2):
      drm/i915: Set the required VFMUNIT clock gating disable on Ironlake.
      drm/i915, intel_ips: When i915 loads after IPS, make IPS relink to i915.

 drivers/gpu/drm/i915/dvo_ch7017.c    |    2 +-
 drivers/gpu/drm/i915/i915_dma.c      |   23 +++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h      |   10 +++++++++
 drivers/gpu/drm/i915/intel_bios.c    |    2 +-
 drivers/gpu/drm/i915/intel_display.c |   21 ++++++++++++++++++-
 drivers/gpu/drm/i915/intel_sdvo.c    |    3 +-
 drivers/platform/x86/intel_ips.c     |   36 +++++++++++++++++++++++++++++++--
 drivers/platform/x86/intel_ips.h     |   21 +++++++++++++++++++
 8 files changed, 111 insertions(+), 7 deletions(-)
 create mode 100644 drivers/platform/x86/intel_ips.h

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [git pull] i915 fixes

[Jesse Barnes]

On Thu, 30 Dec 2010 19:01:34 +0000
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> Hi Linus,
> 
> Dave asked to me send this pull request directly to you as he is on
> holiday over the New Year. In this batch, there are a few regression
> fixes. The most notable being the revert of the SSC frequency switch,
> along with a use-before-initialisation introduced much earlier in the
> sdvo/hdmi audio enabling and a fix for the loss of correct modesetting on
> one particular DVO chipset. Eric also found another instance where we were
> not adhering to the specs when setting up Ironlake. The impact is not
> conclusively known but we did find a few scenarios where it delayed a
> hang.
> -Chris
> 
> The following changes since commit 387c31c7e5c9805b0aef8833d1731a5fe7bdea14:
> 
>   Linux 2.6.37-rc8 (2010-12-28 17:05:48 -0800)
> 
> are available in the git repository at:
>   git://git.kernel.org/pub/scm/linux/kernel/git/ickle/drm-intel.git drm-intel-fixes
> 
> Chris Wilson (4):
>       drm/i915/sdvo: Add hdmi connector properties after initing the connector
>       drm/i915: Verify Ironlake eDP presence on DP_A using the capability fuse
>       Revert "drm/i915/bios: Reverse order of 100/120 Mhz SSC clocks"
>       drm/i915/dvo: Report LVDS attached to ch701x as connected

Did you want to disable SSC entirely on gen5+ just so we can get all
the known machines working?  It's possible it could add some wifi or
sound interference, but that's better than not having a display (most
of the time).

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [git pull] i915 fixes

[Chris Wilson]

On Thu, 30 Dec 2010 14:01:03 -0800, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> Did you want to disable SSC entirely on gen5+ just so we can get all
> the known machines working?  It's possible it could add some wifi or
> sound interference, but that's better than not having a display (most
> of the time).

My first step was to simply restore the behaviour to 2.3.36 and then look
for an alternate patch. I'd like to give any such patch at least the
chance for smoketesting by QA (and the machines here) before asking
Linus to pull ;-)

There is one interesting difference between the BIOS and KMS clock setting
for the Lenovo U160; the BIOS is able to set n=1, whereas we can't find a
suitable clock with just a single wire. Maybe we can fix the U160 and save
0.1W as well!

As far as I am aware, only the LVDS panel on the U160 is broken. Does
anyone else have a panel that sprang to life (ignoring the recent bug)
by disabling SSC?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [git pull] i915 fixes

[Linus Torvalds]

On Thu, Dec 30, 2010 at 2:01 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>
> Did you want to disable SSC entirely on gen5+ just so we can get all
> the known machines working?  It's possible it could add some wifi or
> sound interference, but that's better than not having a display (most
> of the time).

It would be sad to possibly add more electrical noise to machines that
already work with it, but at the same time I do think that "working
screen" tends to be a lot more important than "try to avoid RF noise".

Also, are you sure it's really the fact that we enable spread-spectrum
that causes this? The code is really confused, and seems to mix up
"lvds_use_ssc" not just with the enabling of SSC, but it also with how
impacts the reference clock itself.

And it impacts the reference clock in really odd ways, that look buggy
and confusing, where the tests are repeated in multiple places: first
to set the reference frequency, and then later to set the bits that
choose the reference input frequency.

In particular, look at how 'refclk' is calculated in
intel_crtc_mode_set(), vs how we actually set the input frequency
later in the function. The two don't actually *match*. That sounds
bogus to me - since it means that the pll values have been calculated
for a reference clock that isn't actually used. No?

Look at the code for the "!is_lvds" case, for example. It uses
"IS_GEN2()" to determine what refclk to use, but then when setting the
PLL_REF_INPUT_xyz bits, we actually take "is_tv" into account - which
the code didn't when it calculated refclk. That strikes me as odd. No?

Now, that shouldn't matter for the LVDS case, but I'm wondering
whether something similar is going on where the conditionals just
don't match up, and we end up calculating the plls for a different
frequency than the one we actually end up _using_.

There's also this very odd special refclock magic for ironlake
limiting that only happens for ssc_freq == 100 when ssc is enabled.
Maybe the problem is in the limiting tables, and the ssc frequency
change just ends up then picking the "wrong" limiter table? So even if
the frequency is correct, and the pll calculations are using that
correct frequency, the 120-vs-100Mhz frequency change ended up
switching the tables around?

                               Linus

Re: [git pull] i915 fixes

[Chris Wilson]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3087 bytes --]

On Thu, 30 Dec 2010 14:29:34 -0800, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Thu, Dec 30, 2010 at 2:01 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> >
> > Did you want to disable SSC entirely on gen5+ just so we can get all
> > the known machines working?  It's possible it could add some wifi or
> > sound interference, but that's better than not having a display (most
> > of the time).
> 
> It would be sad to possibly add more electrical noise to machines that
> already work with it, but at the same time I do think that "working
> screen" tends to be a lot more important than "try to avoid RF noise".
> 
> Also, are you sure it's really the fact that we enable spread-spectrum
> that causes this? The code is really confused, and seems to mix up
> "lvds_use_ssc" not just with the enabling of SSC, but it also with how
> impacts the reference clock itself.
> 
> And it impacts the reference clock in really odd ways, that look buggy
> and confusing, where the tests are repeated in multiple places: first
> to set the reference frequency, and then later to set the bits that
> choose the reference input frequency.
> 
> In particular, look at how 'refclk' is calculated in
> intel_crtc_mode_set(), vs how we actually set the input frequency
> later in the function. The two don't actually *match*. That sounds
> bogus to me - since it means that the pll values have been calculated
> for a reference clock that isn't actually used. No?
> 
> Look at the code for the "!is_lvds" case, for example. It uses
> "IS_GEN2()" to determine what refclk to use, but then when setting the
> PLL_REF_INPUT_xyz bits, we actually take "is_tv" into account - which
> the code didn't when it calculated refclk. That strikes me as odd. No?
> 
> Now, that shouldn't matter for the LVDS case, but I'm wondering
> whether something similar is going on where the conditionals just
> don't match up, and we end up calculating the plls for a different
> frequency than the one we actually end up _using_.
> 
> There's also this very odd special refclock magic for ironlake
> limiting that only happens for ssc_freq == 100 when ssc is enabled.
> Maybe the problem is in the limiting tables, and the ssc frequency
> change just ends up then picking the "wrong" limiter table? So even if
> the frequency is correct, and the pll calculations are using that
> correct frequency, the 120-vs-100Mhz frequency change ended up
> switching the tables around?

Switching those tables around was the reason why the one-line change had
any impact at all, and I hoped that it was the leniency in our pll finder
that enabled us to bring up any SSC-enabled panel. I also made one pass at
removing the confusion surrounding refclk, which wasn't -fixes material.
Now I have several more concerns about the code.

However, switching the SSC refclk back to 100MHz we do end up choosing the
same PLL clocks as the BIOS does on the U160, but modesetting still fails.
So the discrepancy is not likely to be in the limit tables themselves.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre