gmane.comp.freedesktop.xorg.drivers.intel

Re: [PATCH 1/3] drm/i915: implement IPS feature

Rodrigo Vivi — 2013-05-18T03:18:07

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

On Thu, May 16, 2013 at 4:54 PM, Paulo Zanoni <przanoni< at >gmail.com> wrote:

Re: [PATCH 2/5] drm/i915: track ring progression usingseqnos

Ben Widawsky — 2013-05-17T17:40:20

Shouldn't you initialize this thing in i915_gem_init_seqno()?

Re: [PATCH 1/2 V3] drm/915: Add private api for powerwell usage

Jesse Barnes — 2013-05-17T15:43:33

A few comments and questions below.

On Thu, 16 May 2013 15:52:36 +0800
Wang Xingchao <xingchao.wang< at >linux.intel.com> wrote:


We can leave the name of this function alone; even for static stuff we
tend to use the intel_ prefix.  Plus it's a set function, not an enable
function...  so maybe just put a __ in front of it to indicate it's for
internal use only.




What does this mean?  If it's just for making sure we don't use bogus
power_well_device, it seems like we can just use a NULL check against
power_well_device for that instead.


I think we should just set the power_well_device at module init time,
then ou wouldn't need to check/set it here.

Also, the existing i915 code could just use the request/release
functions too (internal versions taking a drm_device *), then you
wouldn't need this special case.


For future proofing, it might be good if these took an enum for the
power well being requested.  Then we could track an array of refcounts
later when we need the additional controls.

But I suppose th

Re: debugging Haswell eDP black screen after S3

Ben Guthro — 2013-05-17T13:26:18

This can be reliably reproduced on this machine, and worked around by
saving the vbestate, and restoring it after the fact:

(in a working state)
vbetool vbestate save > vbe.save

break the system:
vbetool dpms off
vbetool dpms on

The following brings the screen back, but in a low resolution corner of X:
vbetool vbestate restore < vbe.save

And then we can get the full resolution back with the following:
xrandr --output eDP1 --off
xrandr --output eDP1 --auto


This is clearly not an ideal solution to make a product out of.

Does this point to a BIOS issue?


Is anyone out there?





_______________________________________________
Intel-gfx mailing list
Intel-gfx< at >lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: debugging Haswell eDP black screen after S3

Ben Guthro — 2013-05-17T11:52:55

It appears that S3 is not necessary.

I can reproduce the black screen with just vbetool:
vbetool dpms off
vbetool dpms on

Does this suggest a bios issue?



_______________________________________________
Intel-gfx mailing list
Intel-gfx< at >lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915: add enable_ips module option

Paulo Zanoni — 2013-05-16T19:56:39

From: Paulo Zanoni <paulo.r.zanoni< at >intel.com>

IPS is still enabled by default. Feature requested by the power
management team.

This should also help testing the feature on some early pre-production
hardware where there were relationship problems between IPS and PSR.

v2: Rebase on top of the newest IPS implementation.

Requested-by: Kristen Accardi <kristen.c.accardi< at >intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni< at >intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      | 4 ++++
 drivers/gpu/drm/i915/i915_drv.h      | 1 +
 drivers/gpu/drm/i915/intel_display.c | 3 ++-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a1a936f..0289f24 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
< at >< at > -128,6 +128,10 < at >< at > module_param_named(disable_power_well, i915_disable_power_well, int, 0600);
 MODULE_PARM_DESC(disable_power_well,
  "Disable the power well when possible (default: false)");
 
+int i915_

[PATCH 1/3] drm/i915: implement IPS feature

Paulo Zanoni — 2013-05-16T19:54:04

From: Paulo Zanoni <paulo.r.zanoni< at >intel.com>

Intermediate Pixel Storage is a feature that should reduce the number
of times the display engine wakes up memory to read pixels, so it
should allow deeper PC states. IPS can only be enabled on ULT pipe A
with 8:8:8 pipe pixel formats.

With eDP 1920x1080 and correct watermarks but without FBC this moves
my PC7 residency from 2.5% to around 38%.

v2: - It's tied to pipe A, not port A
    - Add pipe_config support (Chris)
    - Add some assertions (Chris)
    - Rebase against latest dinq

Signed-off-by: Paulo Zanoni <paulo.r.zanoni< at >intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      | 11 +++++
 drivers/gpu/drm/i915/intel_display.c | 79 +++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |  2 +
 3 files changed, 90 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 32eb97c..65339a7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
< at >< at > -977,6 +97

Re: debugging Haswell eDP black screen after S3

Ben Guthro — 2013-05-16T13:24:05


Below is a serial dump with drm.debug=4, after resuming from S3

If anyone sees anything awry, being pointed in the right direction
would be appreciated:


[  119.676134] ACPI: Low-level resume complete
[  119.676200] PM: Restoring platform NVS memory
[  119.676585] xen-acpi-processor: Uploading Xen processor PM info
[  119.678302] Enabling non-boot CPUs ...
[  119.678351] installing Xen timer for CPU 1
[  119.678380] cpu 1 spinlock event irq 48
[  119.679469] CPU1 is up
[  119.679496] installing Xen timer for CPU 2
[  119.679505] cpu 2 spinlock event irq 55
[  119.680524] CPU2 is up
[  119.680586] installing Xen timer for CPU 3
[  119.680590] cpu 3 spinlock event irq 62
[  119.681463] CPU3 is up
[  119.681482] installing Xen timer for CPU 4
[  119.681487] cpu 4 spinlock event irq 69
[  119.682448] CPU4 is up
[  119.682478] installing Xen timer for CPU 5
[  119.682482] cpu 5 spinlock event irq 76
[  119.683463] CPU5 is up
[  119.683490] installing Xen timer for CPU 6
[  119.683494] cpu 6 spinlock event irq

Re: [PATCH 0/2 V3] Power-well API implementation forHaswell

Wang, Xingchao — 2013-05-16T12:06:00

Hi Daniel,

Do you have further comments on this patchset?

thanks
--xingchao

[PATCH 4/4] drm/i915: remove unused is_cpu_edp()

Imre Deak — 2013-05-16T11:40:37

Signed-off-by: Imre Deak <imre.deak< at >intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |   16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1a4c103..0d326e2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
< at >< at > -59,22 +59,6 < at >< at > static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
 return intel_dig_port->base.base.dev;
 }
 
-/**
- * is_cpu_edp - is the port on the CPU and attached to an eDP panel?
- * < at >intel_dp: DP struct
- *
- * Returns true if the given DP struct corresponds to a CPU eDP port.
- */
-static bool is_cpu_edp(struct intel_dp *intel_dp)
-{
-struct drm_device *dev = intel_dp_to_dev(intel_dp);
-struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-enum port port = intel_dig_port->port;
-
-return is_edp(intel_dp) &&
-(port == PORT_A || (port == PORT_C && IS_VALLEYVIEW(dev)));
-}
-
 static struct intel_dp *intel_attached_dp(struct drm_connector

[PATCH 3/4] drm/i915: replace is_cpu_edp() with a checkfor port A

Imre Deak — 2013-05-16T11:40:36

The patch changes all remaining is_cpu_edp() check with a check for port
A. We can do this, since in all these cases ValleyView is handled
separately and port A is always a CPU side eDP port.

Signed-off-by: Imre Deak <imre.deak< at >intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |   49 +++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b99da13..1a4c103 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
< at >< at > -684,6 +684,7 < at >< at > intel_dp_compute_config(struct intel_encoder *encoder,
 struct drm_i915_private *dev_priv = dev->dev_private;
 struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
 struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+enum port port = dp_to_dig_port(intel_dp)->port;
 struct intel_crtc *intel_crtc = encoder->new_crtc;
 struct intel_connector *intel_connector = intel_dp->attached_connector;
 int lane_count,

[PATCH 2/4] drm/i915: merge VLV eDP and DP AUX clockdivider calculation

Imre Deak — 2013-05-16T11:40:35

On ValleyView for both eDP and DP the AUX input clock is 200MHz, so we
can calculate for both the clock divider for the 2MHz target rate at the
same place. Afterwards we can also replace the is_cpu_edp() check with a
check for port A.

Signed-off-by: Imre Deak <imre.deak< at >intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 90ae58a..b99da13 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
< at >< at > -317,11 +317,11 < at >< at > intel_dp_aux_ch(struct intel_dp *intel_dp,
  * Note that PCH attached eDP panels should use a 125MHz input
  * clock divider.
  */
-if (is_cpu_edp(intel_dp)) {
+if (IS_VALLEYVIEW(dev)) {
+aux_clock_divider = 100;
+} else if (intel_dig_port->port == PORT_A) {
 if (HAS_DDI(dev))
 aux_clock_divider = intel_ddi_get_cdclk_freq(dev_priv) >> 1;
-else if (IS_VALLEYVIEW(dev))
-aux_clock_divider = 100;
 else if (IS_GEN

[PATCH 1/4] drm/i915: stop using is_cpu_edp() inintel_disable/post_disable_dp

Imre Deak — 2013-05-16T11:40:34

On port A and for Valleyview on port C we can have only eDP and in both
cases it's a CPU port. So we can replace is_cpu_edp() with a port check
for these two cases. This allows us to remove is_cpu_edp() completely in
a later patch.

Signed-off-by: Imre Deak <imre.deak< at >intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4dae01a..90ae58a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
< at >< at > -1350,6 +1350,8 < at >< at > static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
 static void intel_disable_dp(struct intel_encoder *encoder)
 {
 struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+enum port port = dp_to_dig_port(intel_dp)->port;
+struct drm_device *dev = encoder->base.dev;
 
 /* Make sure the panel is off before trying to change the mode. But also
  * ensure that we have vdd while we switch off the panel. */
< at >< at > -1

[PATCH 0/4] drm/i915: remove is_cpu_edp()

Imre Deak — 2013-05-16T11:40:33

is_cpu_edp() is equivalent with a port==PORT_A check. There are two
exceptions (see patch 1 and 2), where we can rewrite things to handle
ValleyView separately as it's done at other places. With these out of
the way we can replace is_cpu_edp() with a simpler port check and
remove is_cpu_edp().

I tested this only on IVB, so before applying we should test it on VLV
at least.

Imre Deak (4):
  drm/i915: stop using is_cpu_edp() in intel_disable/post_disable_dp
  drm/i915: merge VLV eDP and DP AUX clock divider calculation
  drm/i915: replace is_cpu_edp() with a check for port A
  drm/i915: remove unused is_cpu_edp()

 drivers/gpu/drm/i915/intel_dp.c |   78 ++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 43 deletions(-)

[PATCH] drm/i915: add support for dvo Chrontel 7010B

2013-05-16T10:57:38

This patch add dvo detection for the Chrontel 7010B on some old hardware.

References: https://bugzilla.kernel.org/show_bug.cgi?id=55101
Signed-off-by: Braggle <braggle at free.fr>
---
 drivers/gpu/drm/i915/dvo_ch7xxx.c |   28 ++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_dvo.c  |    7 +++++++
 2 files changed, 33 insertions(+), 2 deletions(-)

diff -up a/drivers/gpu/drm/i915/dvo_ch7xxx.c b/drivers/gpu/drm/i915/dvo_ch7xxx.c
--- a/drivers/gpu/drm/i915/dvo_ch7xxx.c
+++ b/drivers/gpu/drm/i915/dvo_ch7xxx.c
< at >< at > -32,12 +32,14 < at >< at > SOFTWARE OR THE USE OR OTHER DEALINGS IN
 #define CH7xxx_REG_DID0x4b
 
 #define CH7011_VID0x83 /* 7010 as well */
+#define CH7010B_VID0x05
 #define CH7009A_VID0x84
 #define CH7009B_VID0x85
 #define CH7301_VID0x95
 
 #define CH7xxx_VID0x84
 #define CH7xxx_DID0x17
+#define CH7010_DID0x16
 
 #define CH7xxx_NUM_REGS0x4c
 
< at >< at > -87,11 +89,20 < at >< at > static struct ch7xxx_id_struct {
 char *name;
 } ch7xxx_ids[] = {
 { CH7011_VID, "CH7011" },
+{ CH7010B_VID, "CH7010B" },

[PATCH 1/2 V3] drm/915: Add private api for power wellusage

Wang Xingchao — 2013-05-16T07:52:36

Haswell Display audio depends on power well in graphic side, it should
request power well before use it and release power well after use.
I915 will not shutdown power well if it detects audio is using.
This patch protects display audio crash for Intel Haswell C3 stepping board.

Signed-off-by: Wang Xingchao <xingchao.wang< at >linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c |   75 +++++++++++++++++++++++++++++++++++----
 include/drm/i915_powerwell.h    |   36 +++++++++++++++++++
 2 files changed, 104 insertions(+), 7 deletions(-)
 create mode 100644 include/drm/i915_powerwell.h

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0f4b46e..88820e1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
< at >< at > -4344,18 +4344,12 < at >< at > bool intel_using_power_well(struct drm_device *dev)
 return true;
 }
 
-void intel_set_power_well(struct drm_device *dev, bool enable)
+static void enable_power_well(struct drm_device *dev, bool enable)
 {
 struct drm_i915_p

Re: [PATCH 2/2] ALSA: hda - Add power-welll support forhaswell HDA

Takashi Iwai — 2013-05-16T05:43:09

At Thu, 16 May 2013 03:51:16 +0000,
Wang, Xingchao wrote:

Yes, but the problem is if the driver without CONFIG_SND_HDA_I915 is
used on Haswell.  Then the driver is loaded as if everything is OK
even though you know it's broken.

If you make this selectable, you should give an error from
hda_i915_init() in the case CONFIG_SND_HDA_i915=n, so that the driver
load shall fail.



Looking back at the code, it looks so.  But you can reduce these open
codes via simple symbol_request().



You already know that i915 is never a module but only built-in in the
above.  Thus request_module() and retry is simply superfluous.



Right, I thought wrongly.  So you just need to call symbol_put() for
the leftover ones.


Takashi

Re: [PATCH 2/2] ALSA: hda - Add power-welll support forhaswell HDA

Wang, Xingchao — 2013-05-16T03:51:16

Hi Takashi,

Thanks your quick feedback, please see my comments below.


Well, for Non-Haswell chips with SND_HDA_I915 compiled in, it do nothing as it has no " AZX_DCAPS_I915_POWERWELL" set, so no power-well API called.
For Haswell chips, SND_HDA_I915 must be enabled otherwise display audio would fail. Anyway I added a note message in the choice to let
user know this option is a *must* one for Haswell chips.


Fixed.



Fixed.


Symbol_get/put will call try_module_get() internally so it think it's not necessary get module reference again.
So in above case, if request_module() finished, we can call symbol_get/put safely, Do you think so?


Is it really necessary here? I assume if the module is live, symbol_get() would be successful.
If the symbol_get fail, the module doesnot exist. Is there any other case? i.e. module is live but symbol_get fail?


Yes, fixed in next version.


Yeah, please correct me if my understanding was wrong: 
Symbol_get should be safe here as it would request module reference.
Symbol_

Re: [PATCH 1/6] drm/i915: add encoder get_configfunction v5

Paulo Zanoni — 2013-05-15T21:45:35

2013/5/14 Jesse Barnes <jbarnes< at >virtuousgeek.org>:

I applied this on my Haswell machine and booted it with both eDP-only
and eDP+DP and now I don't see those error messages anymore.

Re: debugging Haswell eDP black screen after S3

Ben Guthro — 2013-05-15T20:42:58

It looks like I misread the code here, inversing an if statement state.

That said, I don't really have any clues as to why the display is
black after resuming from S3

Is this an eDP training issue? Are there any changesets I can try backporting?
I tried this, but it didn't seem to help:
https://patchwork.kernel.org/patch/2516601/
_______________________________________________
Intel-gfx mailing list
Intel-gfx< at >lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC PATCH 2/3] drm/i915: refactor all vlv sidebandaccessors to use one helper

Jani Nikula — 2013-05-15T18:40:31

Based on work by Shobhit Kumar <shobhit.kumar< at >intel.com> and
Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu< at >intel.com>

Signed-off-by: Jani Nikula <jani.nikula< at >intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h       |   93 ++++++++++++++----------------
 drivers/gpu/drm/i915/intel_sideband.c |  102 ++++++++++++++++-----------------
 2 files changed, 93 insertions(+), 102 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7af7ae6..fcee92b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
< at >< at > -344,27 +344,54 < at >< at >
 #define  DEBUG_RESET_DISPLAY(1<<9)
 
 /*
- * DPIO - a special bus for various display related registers to hide behind:
- *  0x800c: m1, m2, n, p1, p2, k dividers
- *  0x8014: REF and SFR select
- *  0x8014: N divider, VCO select
- *  0x801c/3c: core clock bits
- *  0x8048/68: low pass filter coefficients
- *  0x8100: fast clock controls
+ * IOSF sideband
+ */
+#define VLV_IOSF_DOORBELL_REQ(VLV_DISPLAY_BASE + 0x2100)
+#d