Re: [Intel-gfx] [Alsa-user] intel-hda: sound via HDMI only when using interlaced modes

2013-10-10 Thread Jani Nikula
On Wed, 09 Oct 2013, Jasper Smet josb...@gmail.com wrote:
 As promissed i added the params to the kernel boot and here's the
 dmesg output with debugging enabled:

 http://sprunge.us/iEQR

Similar dmesg for when you connect directly to the tv (and audio works)
might prove useful.

Is your expectation that the VSX-928 picks up the sound, or pass through
to the tv? Is it possible to try both (I honestly don't know)?

It might prove useful to file a bug on DRM/Intel at [1] to track
this. Then it won't get lost even if this thread quiets down.


Thanks,
Jani.


[1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI



 Hope this helps!

 On Wed, Oct 9, 2013 at 1:28 PM, Jasper Smet josb...@gmail.com wrote:
 Ok will pass the info when i get home later this day.

 On Wed, Oct 9, 2013 at 1:01 PM, Daniel Vetter dan...@ffwll.ch wrote:
 On Wed, Oct 09, 2013 at 12:44:10PM +0200, Jasper Smet wrote:
 Sorry,

 Intel NUC NUC Kit DC3217BY (Ivy bridge HD4000) Connected via HDMI to
 my Pioneer VSX-928 which is then connected to my Panasonic tv

 Running latest stable Openelec build (3.2.2). (tried different builds
 using stable/unstable drivers)

 That's really old ... Can you please test on something more modern? We
 made tons of fixes to the hdmi infoframe code.

 Here are some logs i pulled while i was debugging the issue with one
 of the OpenElec developers.

 xbmc log (shows ALSA enumeration): http://sprunge.us/YJHc
 alsa playback devices: http://sprunge.us/KZKX
 xrandr modes: http://sprunge.us/DhFK
 dmesg: http://sprunge.us/XRGP

 When you have a new kernel please boot with drm.debug=0xe added to your
 kernel cmdline and then grab the dmesg. Otherwise all the useful gfx debug
 noise isn't in there.
 -Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch



 --
 Met Vriendelijke Groeten

 Jasper Smet
 Developer

 Twitter: josbeir
 E-mail: josb...@gmail.com
 Mobile: 0486/41.75.45



 -- 
 Met Vriendelijke Groeten

 Jasper Smet
 Developer

 Twitter: josbeir
 E-mail: josb...@gmail.com
 Mobile: 0486/41.75.45
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [Alsa-user] intel-hda: sound via HDMI only when using interlaced modes

2013-10-10 Thread Jasper Smet
I managed do this quickly before i got off to work:

Dmesg output with directly connected to the tv: http://sprunge.us/EhJD

Neither the VSX-928 nor the TV (passtrough) pick up sound when
connected to the AV receiver.

I noticed that when i set the resolution to 1080i@30 sound works on
the receiver but not on the TV, everything lower works fine in both
passtrough and with the receiver powered on.

Only when using 1080p24 or lower everything then works fine on both devices..

Hope this helps.

On Thu, Oct 10, 2013 at 7:57 AM, Jani Nikula
jani.nik...@linux.intel.com wrote:
 On Wed, 09 Oct 2013, Jasper Smet josb...@gmail.com wrote:
 As promissed i added the params to the kernel boot and here's the
 dmesg output with debugging enabled:

 http://sprunge.us/iEQR

 Similar dmesg for when you connect directly to the tv (and audio works)
 might prove useful.

 Is your expectation that the VSX-928 picks up the sound, or pass through
 to the tv? Is it possible to try both (I honestly don't know)?

 It might prove useful to file a bug on DRM/Intel at [1] to track
 this. Then it won't get lost even if this thread quiets down.


 Thanks,
 Jani.


 [1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI



 Hope this helps!

 On Wed, Oct 9, 2013 at 1:28 PM, Jasper Smet josb...@gmail.com wrote:
 Ok will pass the info when i get home later this day.

 On Wed, Oct 9, 2013 at 1:01 PM, Daniel Vetter dan...@ffwll.ch wrote:
 On Wed, Oct 09, 2013 at 12:44:10PM +0200, Jasper Smet wrote:
 Sorry,

 Intel NUC NUC Kit DC3217BY (Ivy bridge HD4000) Connected via HDMI to
 my Pioneer VSX-928 which is then connected to my Panasonic tv

 Running latest stable Openelec build (3.2.2). (tried different builds
 using stable/unstable drivers)

 That's really old ... Can you please test on something more modern? We
 made tons of fixes to the hdmi infoframe code.

 Here are some logs i pulled while i was debugging the issue with one
 of the OpenElec developers.

 xbmc log (shows ALSA enumeration): http://sprunge.us/YJHc
 alsa playback devices: http://sprunge.us/KZKX
 xrandr modes: http://sprunge.us/DhFK
 dmesg: http://sprunge.us/XRGP

 When you have a new kernel please boot with drm.debug=0xe added to your
 kernel cmdline and then grab the dmesg. Otherwise all the useful gfx debug
 noise isn't in there.
 -Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch



 --
 Met Vriendelijke Groeten

 Jasper Smet
 Developer

 Twitter: josbeir
 E-mail: josb...@gmail.com
 Mobile: 0486/41.75.45



 --
 Met Vriendelijke Groeten

 Jasper Smet
 Developer

 Twitter: josbeir
 E-mail: josb...@gmail.com
 Mobile: 0486/41.75.45
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

 --
 Jani Nikula, Intel Open Source Technology Center



-- 
Met Vriendelijke Groeten

Jasper Smet
Developer

Twitter: josbeir
E-mail: josb...@gmail.com
Mobile: 0486/41.75.45
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Export the eLLC size from KMD via IOCTL

2013-10-10 Thread Daniel Vetter
On Thu, Oct 10, 2013 at 03:11:40AM +, Guo, Yejun wrote:
 Hi Daniel,
 
 For example, the close source UMD driver also requires this information.

For upstream a userspace blob as consumer isn't good enough, Dave Airlie
has clearly established this precendence in a bunch of loud emails.

So I need something which is open-source as a consumer of this.
-Daniel

 
 Thanks for the reminder, please see the new patch below generated with git 
 format-patch.
 
 From 6033637a040eeb8ae0f31bdbca8d9c988582f99d Mon Sep 17 00:00:00 2001
 From: GuoYejun yejun@intel.com
 Date: Fri, 27 Sep 2013 16:26:18 +0800
 Subject: [PATCH] drm/i915: Export the eLLC size from KMD via IOCTL
 
 The usermode driver needs the eLLC information in order to
 make decisions which can result in significant performance improvements.
 Since all userspace drivers require this same information,
 it makes sense to store it in one globally accessible place, the kernel.
 ---
  drivers/gpu/drm/i915/i915_dma.c |3 +++
  include/uapi/drm/i915_drm.h |1 +
  2 files changed, 4 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
 index dd76d93..4ce36f4 100644
 --- a/drivers/gpu/drm/i915/i915_dma.c
 +++ b/drivers/gpu/drm/i915/i915_dma.c
 @@ -1003,6 +1003,9 @@ static int i915_getparam(struct drm_device *dev, void 
 *data,
   case I915_PARAM_HAS_EXEC_HANDLE_LUT:
   value = 1;
   break;
 + case I915_PARAM_ELLC_SIZE:
 + value = dev_priv-ellc_size;
 + break;
   default:
   DRM_DEBUG(Unknown parameter %d\n, param-param);
   return -EINVAL;
 diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
 index 3a4e97b..a0d4e00 100644
 --- a/include/uapi/drm/i915_drm.h
 +++ b/include/uapi/drm/i915_drm.h
 @@ -335,6 +335,7 @@ typedef struct drm_i915_irq_wait {
  #define I915_PARAM_HAS_EXEC_NO_RELOC  25
  #define I915_PARAM_HAS_EXEC_HANDLE_LUT   26
  #define I915_PARAM_HAS_WT 27
 +#define I915_PARAM_ELLC_SIZE  28
  
  typedef struct drm_i915_getparam {
   int param;
 -- 
 1.7.9.5
 
 
 Thanks
 Yejun
 
 -Original Message-
 From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
 Sent: Wednesday, October 09, 2013 15:22
 To: Guo, Yejun
 Cc: intel-gfx@lists.freedesktop.org
 Subject: Re: [Intel-gfx] [PATCH] drm/i915: Export the eLLC size from KMD via 
 IOCTL
 
 On Wed, Oct 09, 2013 at 12:06:06AM +, Guo, Yejun wrote:
  commit 6033637a040eeb8ae0f31bdbca8d9c988582f99d
  Author: GuoYejun yejun@intel.commailto:yejun@intel.com
  Date:   Fri Sep 27 16:26:18 2013 +0800
  
  drm/i915: Export the eLLC size from KMD via IOCTL
  
  The usermode driver needs the eLLC information in order to
  make decisions which can result in significant performance improvements.
  Since all userspace drivers require this same information,
  it makes sense to store it in one globally accessible place, the kernel.
 
 Do we have some examples of such userspace lying around?
 
 Also, your patch is missing the sob line and doesn't look like it's formatted 
 with git format-patch.
 
 Cheers, Daniel
  
  diff --git a/drivers/gpu/drm/i915/i915_dma.c 
  b/drivers/gpu/drm/i915/i915_dma.c index dd76d93..4ce36f4 100644
  --- a/drivers/gpu/drm/i915/i915_dma.c
  +++ b/drivers/gpu/drm/i915/i915_dma.c
  @@ -1003,6 +1003,9 @@ static int i915_getparam(struct drm_device *dev, void 
  *data,
  case I915_PARAM_HAS_EXEC_HANDLE_LUT:
  value = 1;
  break;
  + case I915_PARAM_ELLC_SIZE:
  + value = dev_priv-ellc_size;
  + break;
  default:
  DRM_DEBUG(Unknown parameter %d\n, 
  param-param);
  return -EINVAL; diff --git 
  a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 
  3a4e97b..a0d4e00 100644
  --- a/include/uapi/drm/i915_drm.h
  +++ b/include/uapi/drm/i915_drm.h
  @@ -335,6 +335,7 @@ typedef struct drm_i915_irq_wait {
  #define I915_PARAM_HAS_EXEC_NO_RELOC25
  #define I915_PARAM_HAS_EXEC_HANDLE_LUT   26
  #define I915_PARAM_HAS_WT 27
  +#define I915_PARAM_ELLC_SIZE   28
  
  typedef struct drm_i915_getparam {
  int param;
  
  
  Yejun
  graphics software engineer
  Tel: (+86) 021-6116-6181
  
 
  ___
  Intel-gfx mailing list
  Intel-gfx@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/intel-gfx
 
 
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org

Re: [Intel-gfx] [Alsa-user] intel-hda: sound via HDMI only when using interlaced modes

2013-10-10 Thread Jani Nikula
On Thu, 10 Oct 2013, Jasper Smet josb...@gmail.com wrote:
 I managed do this quickly before i got off to work:

 Dmesg output with directly connected to the tv: http://sprunge.us/EhJD

 Neither the VSX-928 nor the TV (passtrough) pick up sound when
 connected to the AV receiver.

 I noticed that when i set the resolution to 1080i@30 sound works on
 the receiver but not on the TV, everything lower works fine in both
 passtrough and with the receiver powered on.

 Only when using 1080p24 or lower everything then works fine on both
 devices..

At a glance, the main difference seems to be:

 [drm:drm_detect_monitor_audio], Monitor has basic audio support
-[drm:drm_edid_to_eld], ELD monitor Panasonic-TV
+[drm:drm_edid_to_eld], ELD monitor VSX-923
 [drm:parse_hdmi_vsdb], HDMI: DVI dual 0, max TMDS clock 190, latency present 0 
0, video latency 0 0, audio latency 0 0
-[drm:drm_edid_to_eld], ELD size 9, SAD count 1
+[drm:drm_edid_to_eld], ELD size 13, SAD count 8

This is getting pretty much to unknown territory for me, so if anyone
knows anything better, please chime in!

I think I'd try using the TV's EDID with the VSX-923 to see if the
ELD/SAD difference makes, uh, a difference. Something like this:

1. connect directly to TV
2. copy /sys/class/drm/card0-HDMI-A-1/edid under /lib/firmware with some
   sensible name, e.g. panasonic-edid. the exact source path may vary
   depending on HDMI port etc.
3. make sure you have CONFIG_DRM_LOAD_EDID_FIRMWARE=y
4. use drm.edid_firmware=HDMI-A-1:panasonic-edid module parameter to
   tell DRM to load the edid from /lib/firmware. again, the connector
   name HDMI-A-1 may very depending on HDMI port you use, do check.
5. connect to VSX-923; this should now use the EDID (and consequently
   ELD/SAD) from the TV

I haven't actually tried this myself. *grin*.

Let us know what happens. Thanks.

BR,
Jani.




 Hope this helps.

 On Thu, Oct 10, 2013 at 7:57 AM, Jani Nikula
 jani.nik...@linux.intel.com wrote:
 On Wed, 09 Oct 2013, Jasper Smet josb...@gmail.com wrote:
 As promissed i added the params to the kernel boot and here's the
 dmesg output with debugging enabled:

 http://sprunge.us/iEQR

 Similar dmesg for when you connect directly to the tv (and audio works)
 might prove useful.

 Is your expectation that the VSX-928 picks up the sound, or pass through
 to the tv? Is it possible to try both (I honestly don't know)?

 It might prove useful to file a bug on DRM/Intel at [1] to track
 this. Then it won't get lost even if this thread quiets down.


 Thanks,
 Jani.


 [1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI



 Hope this helps!

 On Wed, Oct 9, 2013 at 1:28 PM, Jasper Smet josb...@gmail.com wrote:
 Ok will pass the info when i get home later this day.

 On Wed, Oct 9, 2013 at 1:01 PM, Daniel Vetter dan...@ffwll.ch wrote:
 On Wed, Oct 09, 2013 at 12:44:10PM +0200, Jasper Smet wrote:
 Sorry,

 Intel NUC NUC Kit DC3217BY (Ivy bridge HD4000) Connected via HDMI to
 my Pioneer VSX-928 which is then connected to my Panasonic tv

 Running latest stable Openelec build (3.2.2). (tried different builds
 using stable/unstable drivers)

 That's really old ... Can you please test on something more modern? We
 made tons of fixes to the hdmi infoframe code.

 Here are some logs i pulled while i was debugging the issue with one
 of the OpenElec developers.

 xbmc log (shows ALSA enumeration): http://sprunge.us/YJHc
 alsa playback devices: http://sprunge.us/KZKX
 xrandr modes: http://sprunge.us/DhFK
 dmesg: http://sprunge.us/XRGP

 When you have a new kernel please boot with drm.debug=0xe added to your
 kernel cmdline and then grab the dmesg. Otherwise all the useful gfx debug
 noise isn't in there.
 -Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch



 --
 Met Vriendelijke Groeten

 Jasper Smet
 Developer

 Twitter: josbeir
 E-mail: josb...@gmail.com
 Mobile: 0486/41.75.45



 --
 Met Vriendelijke Groeten

 Jasper Smet
 Developer

 Twitter: josbeir
 E-mail: josb...@gmail.com
 Mobile: 0486/41.75.45
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

 --
 Jani Nikula, Intel Open Source Technology Center



 -- 
 Met Vriendelijke Groeten

 Jasper Smet
 Developer

 Twitter: josbeir
 E-mail: josb...@gmail.com
 Mobile: 0486/41.75.45

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Populate primary_disabled in intel_modeset_readout_hw_state()

2013-10-10 Thread Ville Syrjälä
On Wed, Oct 09, 2013 at 05:24:57PM +0300, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä ville.syrj...@linux.intel.com
 
 Make sure our primary_disabled matches our expectations after driver
 init.
 
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70270
 Tested-by: Paulo Zanoni paulo.r.zan...@intel.com
 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com

Also
Tested-by: shui yangwei yangweix.s...@intel.com

 ---
  drivers/gpu/drm/i915/intel_display.c | 1 +
  1 file changed, 1 insertion(+)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 1816c46..debbd06 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -10702,6 +10702,7 @@ static void intel_modeset_readout_hw_state(struct 
 drm_device *dev)
crtc-config);
  
   crtc-base.enabled = crtc-active;
 + crtc-primary_disabled = !crtc-active;
  
   DRM_DEBUG_KMS([CRTC:%d] hw state readout: %s\n,
 crtc-base.base.id,
 -- 
 1.8.1.5
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [Alsa-user] intel-hda: sound via HDMI only when using interlaced modes

2013-10-10 Thread Jani Nikula
On Thu, 10 Oct 2013, Jani Nikula jani.nik...@linux.intel.com wrote:
 On Thu, 10 Oct 2013, Jasper Smet josb...@gmail.com wrote:
 I managed do this quickly before i got off to work:

 Dmesg output with directly connected to the tv: http://sprunge.us/EhJD

 Neither the VSX-928 nor the TV (passtrough) pick up sound when
 connected to the AV receiver.

 I noticed that when i set the resolution to 1080i@30 sound works on
 the receiver but not on the TV, everything lower works fine in both
 passtrough and with the receiver powered on.

 Only when using 1080p24 or lower everything then works fine on both
 devices..

 At a glance, the main difference seems to be:

  [drm:drm_detect_monitor_audio], Monitor has basic audio support
 -[drm:drm_edid_to_eld], ELD monitor Panasonic-TV
 +[drm:drm_edid_to_eld], ELD monitor VSX-923
  [drm:parse_hdmi_vsdb], HDMI: DVI dual 0, max TMDS clock 190, latency present 
 0 0, video latency 0 0, audio latency 0 0
 -[drm:drm_edid_to_eld], ELD size 9, SAD count 1
 +[drm:drm_edid_to_eld], ELD size 13, SAD count 8

 This is getting pretty much to unknown territory for me, so if anyone
 knows anything better, please chime in!

So my completely uneducated hunch was that the VSX-923 advertizes audio
capabilities for better quality than the TV, and together the video and
audio oversubscribe the link, unless you degrade video quality. And my
idea below is to use whatever the TV uses. I don't know if we have any
knobs to choose the audio quality somewhere, or whether the hda driver
has something for that. And then again I might be completely off here...

Jani.



 I think I'd try using the TV's EDID with the VSX-923 to see if the
 ELD/SAD difference makes, uh, a difference. Something like this:

 1. connect directly to TV
 2. copy /sys/class/drm/card0-HDMI-A-1/edid under /lib/firmware with some
sensible name, e.g. panasonic-edid. the exact source path may vary
depending on HDMI port etc.
 3. make sure you have CONFIG_DRM_LOAD_EDID_FIRMWARE=y
 4. use drm.edid_firmware=HDMI-A-1:panasonic-edid module parameter to
tell DRM to load the edid from /lib/firmware. again, the connector
name HDMI-A-1 may very depending on HDMI port you use, do check.
 5. connect to VSX-923; this should now use the EDID (and consequently
ELD/SAD) from the TV

 I haven't actually tried this myself. *grin*.

 Let us know what happens. Thanks.

 BR,
 Jani.




 Hope this helps.

 On Thu, Oct 10, 2013 at 7:57 AM, Jani Nikula
 jani.nik...@linux.intel.com wrote:
 On Wed, 09 Oct 2013, Jasper Smet josb...@gmail.com wrote:
 As promissed i added the params to the kernel boot and here's the
 dmesg output with debugging enabled:

 http://sprunge.us/iEQR

 Similar dmesg for when you connect directly to the tv (and audio works)
 might prove useful.

 Is your expectation that the VSX-928 picks up the sound, or pass through
 to the tv? Is it possible to try both (I honestly don't know)?

 It might prove useful to file a bug on DRM/Intel at [1] to track
 this. Then it won't get lost even if this thread quiets down.


 Thanks,
 Jani.


 [1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI



 Hope this helps!

 On Wed, Oct 9, 2013 at 1:28 PM, Jasper Smet josb...@gmail.com wrote:
 Ok will pass the info when i get home later this day.

 On Wed, Oct 9, 2013 at 1:01 PM, Daniel Vetter dan...@ffwll.ch wrote:
 On Wed, Oct 09, 2013 at 12:44:10PM +0200, Jasper Smet wrote:
 Sorry,

 Intel NUC NUC Kit DC3217BY (Ivy bridge HD4000) Connected via HDMI to
 my Pioneer VSX-928 which is then connected to my Panasonic tv

 Running latest stable Openelec build (3.2.2). (tried different builds
 using stable/unstable drivers)

 That's really old ... Can you please test on something more modern? We
 made tons of fixes to the hdmi infoframe code.

 Here are some logs i pulled while i was debugging the issue with one
 of the OpenElec developers.

 xbmc log (shows ALSA enumeration): http://sprunge.us/YJHc
 alsa playback devices: http://sprunge.us/KZKX
 xrandr modes: http://sprunge.us/DhFK
 dmesg: http://sprunge.us/XRGP

 When you have a new kernel please boot with drm.debug=0xe added to your
 kernel cmdline and then grab the dmesg. Otherwise all the useful gfx 
 debug
 noise isn't in there.
 -Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch



 --
 Met Vriendelijke Groeten

 Jasper Smet
 Developer

 Twitter: josbeir
 E-mail: josb...@gmail.com
 Mobile: 0486/41.75.45



 --
 Met Vriendelijke Groeten

 Jasper Smet
 Developer

 Twitter: josbeir
 E-mail: josb...@gmail.com
 Mobile: 0486/41.75.45
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

 --
 Jani Nikula, Intel Open Source Technology Center



 -- 
 Met Vriendelijke Groeten

 Jasper Smet
 Developer

 Twitter: josbeir
 E-mail: josb...@gmail.com
 Mobile: 0486/41.75.45

 -- 
 Jani Nikula, Intel Open Source Technology 

Re: [Intel-gfx] [Alsa-user] intel-hda: sound via HDMI only when using interlaced modes

2013-10-10 Thread Jasper Smet
Ok, for the distro (OpenElec) i'm using i'll need to to re-compile the
kernel with the CONFIG_DRM_LOAD_EDID_FIRMWARE=y option so i'll try to
set up a build env later this day and hope for the best :-)

On Thu, Oct 10, 2013 at 10:27 AM, Jani Nikula
jani.nik...@linux.intel.com wrote:
 On Thu, 10 Oct 2013, Jani Nikula jani.nik...@linux.intel.com wrote:
 On Thu, 10 Oct 2013, Jasper Smet josb...@gmail.com wrote:
 I managed do this quickly before i got off to work:

 Dmesg output with directly connected to the tv: http://sprunge.us/EhJD

 Neither the VSX-928 nor the TV (passtrough) pick up sound when
 connected to the AV receiver.

 I noticed that when i set the resolution to 1080i@30 sound works on
 the receiver but not on the TV, everything lower works fine in both
 passtrough and with the receiver powered on.

 Only when using 1080p24 or lower everything then works fine on both
 devices..

 At a glance, the main difference seems to be:

  [drm:drm_detect_monitor_audio], Monitor has basic audio support
 -[drm:drm_edid_to_eld], ELD monitor Panasonic-TV
 +[drm:drm_edid_to_eld], ELD monitor VSX-923
  [drm:parse_hdmi_vsdb], HDMI: DVI dual 0, max TMDS clock 190, latency 
 present 0 0, video latency 0 0, audio latency 0 0
 -[drm:drm_edid_to_eld], ELD size 9, SAD count 1
 +[drm:drm_edid_to_eld], ELD size 13, SAD count 8

 This is getting pretty much to unknown territory for me, so if anyone
 knows anything better, please chime in!

 So my completely uneducated hunch was that the VSX-923 advertizes audio
 capabilities for better quality than the TV, and together the video and
 audio oversubscribe the link, unless you degrade video quality. And my
 idea below is to use whatever the TV uses. I don't know if we have any
 knobs to choose the audio quality somewhere, or whether the hda driver
 has something for that. And then again I might be completely off here...

 Jani.



 I think I'd try using the TV's EDID with the VSX-923 to see if the
 ELD/SAD difference makes, uh, a difference. Something like this:

 1. connect directly to TV
 2. copy /sys/class/drm/card0-HDMI-A-1/edid under /lib/firmware with some
sensible name, e.g. panasonic-edid. the exact source path may vary
depending on HDMI port etc.
 3. make sure you have CONFIG_DRM_LOAD_EDID_FIRMWARE=y
 4. use drm.edid_firmware=HDMI-A-1:panasonic-edid module parameter to
tell DRM to load the edid from /lib/firmware. again, the connector
name HDMI-A-1 may very depending on HDMI port you use, do check.
 5. connect to VSX-923; this should now use the EDID (and consequently
ELD/SAD) from the TV

 I haven't actually tried this myself. *grin*.

 Let us know what happens. Thanks.

 BR,
 Jani.




 Hope this helps.

 On Thu, Oct 10, 2013 at 7:57 AM, Jani Nikula
 jani.nik...@linux.intel.com wrote:
 On Wed, 09 Oct 2013, Jasper Smet josb...@gmail.com wrote:
 As promissed i added the params to the kernel boot and here's the
 dmesg output with debugging enabled:

 http://sprunge.us/iEQR

 Similar dmesg for when you connect directly to the tv (and audio works)
 might prove useful.

 Is your expectation that the VSX-928 picks up the sound, or pass through
 to the tv? Is it possible to try both (I honestly don't know)?

 It might prove useful to file a bug on DRM/Intel at [1] to track
 this. Then it won't get lost even if this thread quiets down.


 Thanks,
 Jani.


 [1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI



 Hope this helps!

 On Wed, Oct 9, 2013 at 1:28 PM, Jasper Smet josb...@gmail.com wrote:
 Ok will pass the info when i get home later this day.

 On Wed, Oct 9, 2013 at 1:01 PM, Daniel Vetter dan...@ffwll.ch wrote:
 On Wed, Oct 09, 2013 at 12:44:10PM +0200, Jasper Smet wrote:
 Sorry,

 Intel NUC NUC Kit DC3217BY (Ivy bridge HD4000) Connected via HDMI to
 my Pioneer VSX-928 which is then connected to my Panasonic tv

 Running latest stable Openelec build (3.2.2). (tried different builds
 using stable/unstable drivers)

 That's really old ... Can you please test on something more modern? We
 made tons of fixes to the hdmi infoframe code.

 Here are some logs i pulled while i was debugging the issue with one
 of the OpenElec developers.

 xbmc log (shows ALSA enumeration): http://sprunge.us/YJHc
 alsa playback devices: http://sprunge.us/KZKX
 xrandr modes: http://sprunge.us/DhFK
 dmesg: http://sprunge.us/XRGP

 When you have a new kernel please boot with drm.debug=0xe added to your
 kernel cmdline and then grab the dmesg. Otherwise all the useful gfx 
 debug
 noise isn't in there.
 -Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch



 --
 Met Vriendelijke Groeten

 Jasper Smet
 Developer

 Twitter: josbeir
 E-mail: josb...@gmail.com
 Mobile: 0486/41.75.45



 --
 Met Vriendelijke Groeten

 Jasper Smet
 Developer

 Twitter: josbeir
 E-mail: josb...@gmail.com
 Mobile: 0486/41.75.45
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 

Re: [Intel-gfx] [PATCH 2/2] drm/i915: Rename primary_disabled to primary_enabled

2013-10-10 Thread Jani Nikula
On Wed, 09 Oct 2013, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 Let's try to avoid these confusing negated booleans.

Thanks for such a positive patch!

For the series,
Reviewed-by: Jani Nikula jani.nik...@intel.com

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/intel_display.c | 10 +-
  drivers/gpu/drm/i915/intel_drv.h |  2 +-
  drivers/gpu/drm/i915/intel_pm.c  |  2 +-
  drivers/gpu/drm/i915/intel_sprite.c  |  8 
  4 files changed, 11 insertions(+), 11 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index debbd06..27f98bc 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -1840,9 +1840,9 @@ static void intel_enable_primary_plane(struct 
 drm_i915_private *dev_priv,
   /* If the pipe isn't enabled, we can't pump pixels and may hang */
   assert_pipe_enabled(dev_priv, pipe);
  
 - WARN(!intel_crtc-primary_disabled, Primary plane already enabled\n);
 + WARN(intel_crtc-primary_enabled, Primary plane already enabled\n);
  
 - intel_crtc-primary_disabled = false;
 + intel_crtc-primary_enabled = true;
  
   reg = DSPCNTR(plane);
   val = I915_READ(reg);
 @@ -1870,9 +1870,9 @@ static void intel_disable_primary_plane(struct 
 drm_i915_private *dev_priv,
   int reg;
   u32 val;
  
 - WARN(intel_crtc-primary_disabled, Primary plane already disabled\n);
 + WARN(!intel_crtc-primary_enabled, Primary plane already disabled\n);
  
 - intel_crtc-primary_disabled = true;
 + intel_crtc-primary_enabled = false;
  
   reg = DSPCNTR(plane);
   val = I915_READ(reg);
 @@ -10702,7 +10702,7 @@ static void intel_modeset_readout_hw_state(struct 
 drm_device *dev)
crtc-config);
  
   crtc-base.enabled = crtc-active;
 - crtc-primary_disabled = !crtc-active;
 + crtc-primary_enabled = crtc-active;
  
   DRM_DEBUG_KMS([CRTC:%d] hw state readout: %s\n,
 crtc-base.base.id,
 diff --git a/drivers/gpu/drm/i915/intel_drv.h 
 b/drivers/gpu/drm/i915/intel_drv.h
 index 944a406..28f3800d 100644
 --- a/drivers/gpu/drm/i915/intel_drv.h
 +++ b/drivers/gpu/drm/i915/intel_drv.h
 @@ -321,7 +321,7 @@ struct intel_crtc {
*/
   bool active;
   bool eld_vld;
 - bool primary_disabled; /* is the crtc obscured by a plane? */
 + bool primary_enabled; /* is the primary plane (partially) visible? */
   bool lowfreq_avail;
   struct intel_overlay *overlay;
   struct intel_unpin_work *unpin_work;
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 77d63cf..271dcb9 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -475,7 +475,7 @@ void intel_update_fbc(struct drm_device *dev)
*/
   list_for_each_entry(tmp_crtc, dev-mode_config.crtc_list, head) {
   if (intel_crtc_active(tmp_crtc) 
 - !to_intel_crtc(tmp_crtc)-primary_disabled) {
 + to_intel_crtc(tmp_crtc)-primary_enabled) {
   if (crtc) {
   if (set_no_fbc_reason(dev_priv, 
 FBC_MULTIPLE_PIPES))
   DRM_DEBUG_KMS(more than one pipe 
 active, disabling compression\n);
 diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
 b/drivers/gpu/drm/i915/intel_sprite.c
 index e001d2c..8afaad6 100644
 --- a/drivers/gpu/drm/i915/intel_sprite.c
 +++ b/drivers/gpu/drm/i915/intel_sprite.c
 @@ -521,10 +521,10 @@ intel_enable_primary(struct drm_crtc *crtc)
   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
   int reg = DSPCNTR(intel_crtc-plane);
  
 - if (!intel_crtc-primary_disabled)
 + if (intel_crtc-primary_enabled)
   return;
  
 - intel_crtc-primary_disabled = false;
 + intel_crtc-primary_enabled = true;
  
   I915_WRITE(reg, I915_READ(reg) | DISPLAY_PLANE_ENABLE);
   intel_flush_primary_plane(dev_priv, intel_crtc-plane);
 @@ -553,10 +553,10 @@ intel_disable_primary(struct drm_crtc *crtc)
   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
   int reg = DSPCNTR(intel_crtc-plane);
  
 - if (intel_crtc-primary_disabled)
 + if (!intel_crtc-primary_enabled)
   return;
  
 - intel_crtc-primary_disabled = true;
 + intel_crtc-primary_enabled = false;
  
   mutex_lock(dev-struct_mutex);
   if (dev_priv-fbc.plane == intel_crtc-plane)
 -- 
 1.8.1.5

 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org

Re: [Intel-gfx] [Alsa-user] intel-hda: sound via HDMI only when using interlaced modes

2013-10-10 Thread Jani Nikula
On Thu, 10 Oct 2013, Jasper Smet josb...@gmail.com wrote:
 Ok, for the distro (OpenElec) i'm using i'll need to to re-compile the
 kernel with the CONFIG_DRM_LOAD_EDID_FIRMWARE=y option so i'll try to
 set up a build env later this day and hope for the best :-)

Heh, good luck!

In the mean time, please send us the EDIDs from both the TV and the AV
receiver, in case we can think of something. It's the same binary file
mentioned in step 2.

BR,
Jani.




 On Thu, Oct 10, 2013 at 10:27 AM, Jani Nikula
 jani.nik...@linux.intel.com wrote:
 On Thu, 10 Oct 2013, Jani Nikula jani.nik...@linux.intel.com wrote:
 On Thu, 10 Oct 2013, Jasper Smet josb...@gmail.com wrote:
 I managed do this quickly before i got off to work:

 Dmesg output with directly connected to the tv: http://sprunge.us/EhJD

 Neither the VSX-928 nor the TV (passtrough) pick up sound when
 connected to the AV receiver.

 I noticed that when i set the resolution to 1080i@30 sound works on
 the receiver but not on the TV, everything lower works fine in both
 passtrough and with the receiver powered on.

 Only when using 1080p24 or lower everything then works fine on both
 devices..

 At a glance, the main difference seems to be:

  [drm:drm_detect_monitor_audio], Monitor has basic audio support
 -[drm:drm_edid_to_eld], ELD monitor Panasonic-TV
 +[drm:drm_edid_to_eld], ELD monitor VSX-923
  [drm:parse_hdmi_vsdb], HDMI: DVI dual 0, max TMDS clock 190, latency 
 present 0 0, video latency 0 0, audio latency 0 0
 -[drm:drm_edid_to_eld], ELD size 9, SAD count 1
 +[drm:drm_edid_to_eld], ELD size 13, SAD count 8

 This is getting pretty much to unknown territory for me, so if anyone
 knows anything better, please chime in!

 So my completely uneducated hunch was that the VSX-923 advertizes audio
 capabilities for better quality than the TV, and together the video and
 audio oversubscribe the link, unless you degrade video quality. And my
 idea below is to use whatever the TV uses. I don't know if we have any
 knobs to choose the audio quality somewhere, or whether the hda driver
 has something for that. And then again I might be completely off here...

 Jani.



 I think I'd try using the TV's EDID with the VSX-923 to see if the
 ELD/SAD difference makes, uh, a difference. Something like this:

 1. connect directly to TV
 2. copy /sys/class/drm/card0-HDMI-A-1/edid under /lib/firmware with some
sensible name, e.g. panasonic-edid. the exact source path may vary
depending on HDMI port etc.
 3. make sure you have CONFIG_DRM_LOAD_EDID_FIRMWARE=y
 4. use drm.edid_firmware=HDMI-A-1:panasonic-edid module parameter to
tell DRM to load the edid from /lib/firmware. again, the connector
name HDMI-A-1 may very depending on HDMI port you use, do check.
 5. connect to VSX-923; this should now use the EDID (and consequently
ELD/SAD) from the TV

 I haven't actually tried this myself. *grin*.

 Let us know what happens. Thanks.

 BR,
 Jani.




 Hope this helps.

 On Thu, Oct 10, 2013 at 7:57 AM, Jani Nikula
 jani.nik...@linux.intel.com wrote:
 On Wed, 09 Oct 2013, Jasper Smet josb...@gmail.com wrote:
 As promissed i added the params to the kernel boot and here's the
 dmesg output with debugging enabled:

 http://sprunge.us/iEQR

 Similar dmesg for when you connect directly to the tv (and audio works)
 might prove useful.

 Is your expectation that the VSX-928 picks up the sound, or pass through
 to the tv? Is it possible to try both (I honestly don't know)?

 It might prove useful to file a bug on DRM/Intel at [1] to track
 this. Then it won't get lost even if this thread quiets down.


 Thanks,
 Jani.


 [1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI



 Hope this helps!

 On Wed, Oct 9, 2013 at 1:28 PM, Jasper Smet josb...@gmail.com wrote:
 Ok will pass the info when i get home later this day.

 On Wed, Oct 9, 2013 at 1:01 PM, Daniel Vetter dan...@ffwll.ch wrote:
 On Wed, Oct 09, 2013 at 12:44:10PM +0200, Jasper Smet wrote:
 Sorry,

 Intel NUC NUC Kit DC3217BY (Ivy bridge HD4000) Connected via HDMI to
 my Pioneer VSX-928 which is then connected to my Panasonic tv

 Running latest stable Openelec build (3.2.2). (tried different builds
 using stable/unstable drivers)

 That's really old ... Can you please test on something more modern? We
 made tons of fixes to the hdmi infoframe code.

 Here are some logs i pulled while i was debugging the issue with one
 of the OpenElec developers.

 xbmc log (shows ALSA enumeration): http://sprunge.us/YJHc
 alsa playback devices: http://sprunge.us/KZKX
 xrandr modes: http://sprunge.us/DhFK
 dmesg: http://sprunge.us/XRGP

 When you have a new kernel please boot with drm.debug=0xe added to your
 kernel cmdline and then grab the dmesg. Otherwise all the useful gfx 
 debug
 noise isn't in there.
 -Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch



 --
 Met Vriendelijke Groeten

 Jasper Smet
 Developer

 Twitter: josbeir
 E-mail: josb...@gmail.com
 

Re: [Intel-gfx] [Alsa-user] intel-hda: sound via HDMI only when using interlaced modes

2013-10-10 Thread Jasper Smet
That will be this evening as i'm at work now :'(

On Thu, Oct 10, 2013 at 10:38 AM, Jani Nikula
jani.nik...@linux.intel.com wrote:
 On Thu, 10 Oct 2013, Jasper Smet josb...@gmail.com wrote:
 Ok, for the distro (OpenElec) i'm using i'll need to to re-compile the
 kernel with the CONFIG_DRM_LOAD_EDID_FIRMWARE=y option so i'll try to
 set up a build env later this day and hope for the best :-)

 Heh, good luck!

 In the mean time, please send us the EDIDs from both the TV and the AV
 receiver, in case we can think of something. It's the same binary file
 mentioned in step 2.

 BR,
 Jani.




 On Thu, Oct 10, 2013 at 10:27 AM, Jani Nikula
 jani.nik...@linux.intel.com wrote:
 On Thu, 10 Oct 2013, Jani Nikula jani.nik...@linux.intel.com wrote:
 On Thu, 10 Oct 2013, Jasper Smet josb...@gmail.com wrote:
 I managed do this quickly before i got off to work:

 Dmesg output with directly connected to the tv: http://sprunge.us/EhJD

 Neither the VSX-928 nor the TV (passtrough) pick up sound when
 connected to the AV receiver.

 I noticed that when i set the resolution to 1080i@30 sound works on
 the receiver but not on the TV, everything lower works fine in both
 passtrough and with the receiver powered on.

 Only when using 1080p24 or lower everything then works fine on both
 devices..

 At a glance, the main difference seems to be:

  [drm:drm_detect_monitor_audio], Monitor has basic audio support
 -[drm:drm_edid_to_eld], ELD monitor Panasonic-TV
 +[drm:drm_edid_to_eld], ELD monitor VSX-923
  [drm:parse_hdmi_vsdb], HDMI: DVI dual 0, max TMDS clock 190, latency 
 present 0 0, video latency 0 0, audio latency 0 0
 -[drm:drm_edid_to_eld], ELD size 9, SAD count 1
 +[drm:drm_edid_to_eld], ELD size 13, SAD count 8

 This is getting pretty much to unknown territory for me, so if anyone
 knows anything better, please chime in!

 So my completely uneducated hunch was that the VSX-923 advertizes audio
 capabilities for better quality than the TV, and together the video and
 audio oversubscribe the link, unless you degrade video quality. And my
 idea below is to use whatever the TV uses. I don't know if we have any
 knobs to choose the audio quality somewhere, or whether the hda driver
 has something for that. And then again I might be completely off here...

 Jani.



 I think I'd try using the TV's EDID with the VSX-923 to see if the
 ELD/SAD difference makes, uh, a difference. Something like this:

 1. connect directly to TV
 2. copy /sys/class/drm/card0-HDMI-A-1/edid under /lib/firmware with some
sensible name, e.g. panasonic-edid. the exact source path may vary
depending on HDMI port etc.
 3. make sure you have CONFIG_DRM_LOAD_EDID_FIRMWARE=y
 4. use drm.edid_firmware=HDMI-A-1:panasonic-edid module parameter to
tell DRM to load the edid from /lib/firmware. again, the connector
name HDMI-A-1 may very depending on HDMI port you use, do check.
 5. connect to VSX-923; this should now use the EDID (and consequently
ELD/SAD) from the TV

 I haven't actually tried this myself. *grin*.

 Let us know what happens. Thanks.

 BR,
 Jani.




 Hope this helps.

 On Thu, Oct 10, 2013 at 7:57 AM, Jani Nikula
 jani.nik...@linux.intel.com wrote:
 On Wed, 09 Oct 2013, Jasper Smet josb...@gmail.com wrote:
 As promissed i added the params to the kernel boot and here's the
 dmesg output with debugging enabled:

 http://sprunge.us/iEQR

 Similar dmesg for when you connect directly to the tv (and audio works)
 might prove useful.

 Is your expectation that the VSX-928 picks up the sound, or pass through
 to the tv? Is it possible to try both (I honestly don't know)?

 It might prove useful to file a bug on DRM/Intel at [1] to track
 this. Then it won't get lost even if this thread quiets down.


 Thanks,
 Jani.


 [1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI



 Hope this helps!

 On Wed, Oct 9, 2013 at 1:28 PM, Jasper Smet josb...@gmail.com wrote:
 Ok will pass the info when i get home later this day.

 On Wed, Oct 9, 2013 at 1:01 PM, Daniel Vetter dan...@ffwll.ch wrote:
 On Wed, Oct 09, 2013 at 12:44:10PM +0200, Jasper Smet wrote:
 Sorry,

 Intel NUC NUC Kit DC3217BY (Ivy bridge HD4000) Connected via HDMI to
 my Pioneer VSX-928 which is then connected to my Panasonic tv

 Running latest stable Openelec build (3.2.2). (tried different builds
 using stable/unstable drivers)

 That's really old ... Can you please test on something more modern? We
 made tons of fixes to the hdmi infoframe code.

 Here are some logs i pulled while i was debugging the issue with one
 of the OpenElec developers.

 xbmc log (shows ALSA enumeration): http://sprunge.us/YJHc
 alsa playback devices: http://sprunge.us/KZKX
 xrandr modes: http://sprunge.us/DhFK
 dmesg: http://sprunge.us/XRGP

 When you have a new kernel please boot with drm.debug=0xe added to 
 your
 kernel cmdline and then grab the dmesg. Otherwise all the useful gfx 
 debug
 noise isn't in there.
 -Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 

[Intel-gfx] [PATCH] drm/i915: Cancel outstanding pc8 work when shutting down the device

2013-10-10 Thread Chris Wilson
As part of the device quiesceing we need to disable all active timers
and delayed workers so that they do not execute after the module is
unloaded.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Paulo Zanoni paulo.r.zan...@intel.com
Cc: Rodrigo Vivi rodrigo.v...@gmail.com
---
 drivers/gpu/drm/i915/intel_display.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 683b68e..5ce1558 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11065,6 +11065,7 @@ void intel_modeset_quiesce(struct drm_device *dev)
 
cancel_work_sync(dev_priv-hotplug_work);
cancel_work_sync(dev_priv-rps.work);
+   cancel_delayed_work_sync(dev_priv-pc8.enable_work);
 
/* catch all required for dev_priv-wq */
flush_scheduled_work();
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 3/4] drm/i915: Use unsigned long for obj-user_pin_count

2013-10-10 Thread Ville Syrjälä
On Wed, Oct 09, 2013 at 09:23:53PM +0200, Daniel Vetter wrote:
 At least on linux sizeof(long) == sizeof(void*) and the thinking
 is that you can grab about as many references as there's memory.

Well, there can be more memory than there is address space.

Unchecked counters always leave me a bit uneasy, so an explicit check
is what I'd prefer to see.

 
 Doesn't really matter, just a bit of OCD since the fixed size data
 type in a pure in-kernel datastructure look off.
 
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/i915_drv.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 7fa017b..f39a6b8 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -1567,7 +1567,7 @@ struct drm_i915_gem_object {
   unsigned long *bit_17;
  
   /** User space pin count and filp owning the pin */
 - uint32_t user_pin_count;
 + unsigned long user_pin_count;
   struct drm_file *pin_filp;
  
   /** for phy allocated objects */
 -- 
 1.8.1.4

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/2] drm/i915: Rename primary_disabled to primary_enabled

2013-10-10 Thread Daniel Vetter
On Thu, Oct 10, 2013 at 11:35:24AM +0300, Jani Nikula wrote:
 On Wed, 09 Oct 2013, ville.syrj...@linux.intel.com wrote:
  From: Ville Syrjälä ville.syrj...@linux.intel.com
 
  Let's try to avoid these confusing negated booleans.
 
 Thanks for such a positive patch!
 
 For the series,
 Reviewed-by: Jani Nikula jani.nik...@intel.com

Both merged, thanks for patchesreview.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 3/4] drm/i915: Use unsigned long for obj-user_pin_count

2013-10-10 Thread Daniel Vetter
On Thu, Oct 10, 2013 at 10:56 AM, Ville Syrjälä
ville.syrj...@linux.intel.com wrote:
 On Wed, Oct 09, 2013 at 09:23:53PM +0200, Daniel Vetter wrote:
 At least on linux sizeof(long) == sizeof(void*) and the thinking
 is that you can grab about as many references as there's memory.

 Well, there can be more memory than there is address space.

 Unchecked counters always leave me a bit uneasy, so an explicit check
 is what I'd prefer to see.

Not for normal kernel references, but this one here is just userspace.
I'll add a UINT_MAX check to the pin ioctl to plug that leak. That'll
also make this patch a notch more useful. I'm not going to write a
testcase for this though since Ben will complain about the runtime of
it ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Capture the initial error-state when kicking stuck rings

2013-10-10 Thread Chris Wilson
We lost the ability to capture the first error for a stuck ring in the
recent hangcheck robustification. Whilst both error states are
interesting (why does the GPU not recover is also essential to debug),
our primary goal is to fix the initial hang and so we need to capture
the first error state upon taking hangcheck action.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_irq.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f8d5ad50e898..3b378b87b734 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1995,6 +1995,7 @@ ring_stuck(struct intel_ring_buffer *ring, u32 acthd)
if (tmp  RING_WAIT) {
DRM_ERROR(Kicking stuck wait on %s\n,
  ring-name);
+   i915_handle_error(dev, false);
I915_WRITE_CTL(ring, tmp);
return HANGCHECK_KICK;
}
@@ -2006,6 +2007,7 @@ ring_stuck(struct intel_ring_buffer *ring, u32 acthd)
case 1:
DRM_ERROR(Kicking stuck semaphore on %s\n,
  ring-name);
+   i915_handle_error(dev, false);
I915_WRITE_CTL(ring, tmp);
return HANGCHECK_KICK;
case 0:
-- 
1.8.4.rc3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Capture the initial error-state when kicking stuck rings

2013-10-10 Thread Mika Kuoppala
Chris Wilson ch...@chris-wilson.co.uk writes:

 We lost the ability to capture the first error for a stuck ring in the
 recent hangcheck robustification. Whilst both error states are
 interesting (why does the GPU not recover is also essential to debug),
 our primary goal is to fix the initial hang and so we need to capture
 the first error state upon taking hangcheck action.

 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk

Reviewed-by: Mika Kuoppala mika.kuopp...@intel.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Capture the initial error-state when kicking stuck rings

2013-10-10 Thread Daniel Vetter
On Thu, Oct 10, 2013 at 12:41:15PM +0300, Mika Kuoppala wrote:
 Chris Wilson ch...@chris-wilson.co.uk writes:
 
  We lost the ability to capture the first error for a stuck ring in the
  recent hangcheck robustification. Whilst both error states are
  interesting (why does the GPU not recover is also essential to debug),
  our primary goal is to fix the initial hang and so we need to capture
  the first error state upon taking hangcheck action.
 
  Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 
 Reviewed-by: Mika Kuoppala mika.kuopp...@intel.com

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 1/2] drm/i915/hsw: Add I915_EXEC_RESOURCE_STREAMER flag (v3)

2013-10-10 Thread Abdiel Janulgue
Ensures that the batch buffer is executed by the resource streamer.

v3: - Make sure batch is only submitted on render ring and Haswell (Daniel)
- Separate EXEC and DISPATCH flags (Chris)
- Update __I915_EXEC_UNKNOWN_FLAGS (Kenneth)

Signed-off-by: Abdiel Janulgue abdiel.janul...@linux.intel.com
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 7 +++
 drivers/gpu/drm/i915/intel_ringbuffer.h| 1 +
 include/uapi/drm/i915_drm.h| 7 ++-
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 0ce0d47..833677d 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -957,6 +957,13 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
}
if (args-flags  I915_EXEC_IS_PINNED)
flags |= I915_DISPATCH_PINNED;
+   if (args-flags  I915_EXEC_RESOURCE_STREAMER) {
+   if ((args-flags  I915_EXEC_RING_MASK) != I915_EXEC_RENDER ||
+   !IS_HASWELL(dev))
+   return -EINVAL;
+
+   flags |= I915_DISPATCH_RS;
+   }
 
switch (args-flags  I915_EXEC_RING_MASK) {
case I915_EXEC_DEFAULT:
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 71a73f4..bd74427 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -110,6 +110,7 @@ struct  intel_ring_buffer {
   unsigned flags);
 #define I915_DISPATCH_SECURE 0x1
 #define I915_DISPATCH_PINNED 0x2
+#define I915_DISPATCH_RS 0x4
void(*cleanup)(struct intel_ring_buffer *ring);
int (*sync_to)(struct intel_ring_buffer *ring,
   struct intel_ring_buffer *to,
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 3a4e97b..edd6e31 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -731,7 +731,12 @@ struct drm_i915_gem_execbuffer2 {
  */
 #define I915_EXEC_HANDLE_LUT   (112)
 
-#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_HANDLE_LUT1)
+/** Tell the kernel that the batchbuffer is processed by
+ *  the resource streamer.
+ */
+#define I915_EXEC_RESOURCE_STREAMER (113)
+
+#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_RESOURCE_STREAMER 1)
 
 #define I915_EXEC_CONTEXT_ID_MASK  (0x)
 #define i915_execbuffer2_set_context_id(eb2, context) \
-- 
1.8.2.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/4] drm/i915: prevent tiling changes on framebuffer backing storage

2013-10-10 Thread Ville Syrjälä
On Thu, Oct 10, 2013 at 12:09:48AM +0200, Daniel Vetter wrote:
 On Thu, Oct 10, 2013 at 12:02:17AM +0200, Daniel Vetter wrote:
  On Wed, Oct 09, 2013 at 10:29:39PM +0100, Chris Wilson wrote:
   On Wed, Oct 09, 2013 at 09:23:52PM +0200, Daniel Vetter wrote:
Assuming that all framebuffer related metadata is invariant simplifies
our userspace input data checking. And current userspace always first
updates the tiling of an object before creating a framebuffer with it.
   
   Userspace already changes the tiling layout whilst keeping the fb id.
  
  How exactly does that work? You can't really change the fb pitch without
  creating a new one ... That leaves switching from tiled to untiled and
  back I think.
 
 Meh, didn't read sna carefully enough. On a second read we only seem to
 have the code added
 
 commit 0dd20381364aabede2e1306945abe21d57c1d7b4
 Author: Chris Wilson ch...@chris-wilson.co.uk
 Date:   Sun Sep 29 11:19:46 2013 +0100
 
 sna: Resize an existing framebuffer if possible
 
 That seems to just be for the cache, should be able to cope with failures
 and we can fix it by moving the rmfb up before the set_tiling. It's also
 only 2 weeks old.
 
 So if there's nothing else I've missed I strongly vote to break the
 pre-release over saner intefaces - allowing tiling to change kinda wreaks
 a bit havoc with out in-kernel checks ...

It would be a bit simpler if we can trust that things don't change
after we've created the fb.

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 2/2] drm/i915/hsw: Enable resource streamer bit on MI_BATCH_BUFFER_START (v3)

2013-10-10 Thread Abdiel Janulgue
v3: Use the I915_DISPATCH_RS flag to determine if batchbuffer needs
resource streamer bit.

Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Daniel Vetter dan...@ffwll.ch
Cc: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Abdiel Janulgue abdiel.janul...@linux.intel.com
---
 drivers/gpu/drm/i915/i915_reg.h | 1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c246727..f3c9103 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -246,6 +246,7 @@
 #define   MI_BATCH_NON_SECURE_HSW  (113)
 #define MI_BATCH_BUFFER_START  MI_INSTR(0x31, 0)
 #define   MI_BATCH_GTT (26) /* aliased with (17) on gen4 */
+#define   MI_BATCH_RESOURCE_STREAMER (110)
 #define MI_SEMAPHORE_MBOX  MI_INSTR(0x16, 1) /* gen6+ */
 #define  MI_SEMAPHORE_GLOBAL_GTT(122)
 #define  MI_SEMAPHORE_UPDATE   (121)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index b67104a..f9564b1 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1654,7 +1654,8 @@ hsw_ring_dispatch_execbuffer(struct intel_ring_buffer 
*ring,
 
intel_ring_emit(ring,
MI_BATCH_BUFFER_START | MI_BATCH_PPGTT_HSW |
-   (flags  I915_DISPATCH_SECURE ? 0 : 
MI_BATCH_NON_SECURE_HSW));
+   (flags  I915_DISPATCH_SECURE ? 0 : 
MI_BATCH_NON_SECURE_HSW) |
+   (flags  I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER 
: 0));
/* bit0-7 is the length on GEN6+ */
intel_ring_emit(ring, offset);
intel_ring_advance(ring);
-- 
1.8.2.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/2] drm/i915/hsw: Enable resource streamer bit on MI_BATCH_BUFFER_START (v3)

2013-10-10 Thread Chris Wilson
On Thu, Oct 10, 2013 at 02:03:00PM +0300, Abdiel Janulgue wrote:
 v3: Use the I915_DISPATCH_RS flag to determine if batchbuffer needs
 resource streamer bit.
 
 Cc: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Daniel Vetter dan...@ffwll.ch
 Cc: Ben Widawsky b...@bwidawsk.net
 Signed-off-by: Abdiel Janulgue abdiel.janul...@linux.intel.com

Note this should be the first patch so that we enable the feature before
we tell userspace it is supported.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t] testdisplay: Allow getopt to print error messages

2013-10-10 Thread Thomas Wood
By not assigning opterr, getopt will print its own error message that
includes information about whether an option is unknown or just requires
an additional argument.

Signed-off-by: Thomas Wood thomas.w...@intel.com
---
 tests/testdisplay.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tests/testdisplay.c b/tests/testdisplay.c
index 0fcce19..a0f43e9 100644
--- a/tests/testdisplay.c
+++ b/tests/testdisplay.c
@@ -723,7 +723,6 @@ int main(int argc, char **argv)
 
enter_exec_path( argv );
 
-   opterr = 0;
while ((c = getopt(argc, argv, optstr)) != -1) {
switch (c) {
case '3':
@@ -771,7 +770,6 @@ int main(int argc, char **argv)
sscanf(optarg, %d,%d, specified_disp_id, 
specified_mode_num);
break;
default:
-   fprintf(stderr, unknown option %c\n, c);
/* fall through */
case 'h':
usage(argv[0]);
-- 
1.8.3.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Use unsigned long for obj-user_pin_count

2013-10-10 Thread Daniel Vetter
At least on linux sizeof(long) == sizeof(void*) and the thinking
is that you can grab about as many references as there's memory.

Doesn't really matter, just a bit of OCD since the fixed size data
type in a pure in-kernel datastructure look off.

v2: Ville asked for an overflow check since no one prevents userspace
from incrementing the pin count forever.

Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_drv.h | 2 +-
 drivers/gpu/drm/i915/i915_gem.c | 5 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7fa017b..f39a6b8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1567,7 +1567,7 @@ struct drm_i915_gem_object {
unsigned long *bit_17;
 
/** User space pin count and filp owning the pin */
-   uint32_t user_pin_count;
+   unsigned long user_pin_count;
struct drm_file *pin_filp;
 
/** for phy allocated objects */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 71dd030..e8271d3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3929,6 +3929,11 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data,
goto out;
}
 
+   if (obj-user_pin_count == UINT_MAX) {
+   ret = -EBUSY;
+   goto out;
+   }
+
if (obj-user_pin_count == 0) {
ret = i915_gem_obj_ggtt_pin(obj, args-alignment, true, false);
if (ret)
-- 
1.8.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 0/4] framebuffer_init checks

2013-10-10 Thread Ville Syrjälä
On Wed, Oct 09, 2013 at 09:23:50PM +0200, Daniel Vetter wrote:
 Hi all,
 
 That little patch turned into a bit more, and on top of it there's now also a
 new testcase in igt: kms_addfb. It checks for most of the evil stuff you can
 feed to addfb.

For the series:
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

 
 Cheers, Daniel
 
 Daniel Vetter (4):
   drm/i915: grab dev-struct_mutex around framebuffer_init
   drm/i915: prevent tiling changes on framebuffer backing storage
   drm/i915: Use unsigned long for obj-user_pin_count
   drm/i915: check gem bo size when creating framebuffers
 
  drivers/gpu/drm/i915/i915_drv.h|  5 -
  drivers/gpu/drm/i915/i915_gem_tiling.c |  2 +-
  drivers/gpu/drm/i915/intel_display.c   | 33 -
  3 files changed, 29 insertions(+), 11 deletions(-)
 
 -- 
 1.8.1.4

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Use unsigned long for obj-user_pin_count

2013-10-10 Thread Chris Wilson
On Thu, Oct 10, 2013 at 01:29:37PM +0200, Daniel Vetter wrote:
 At least on linux sizeof(long) == sizeof(void*) and the thinking
 is that you can grab about as many references as there's memory.
 
 Doesn't really matter, just a bit of OCD since the fixed size data
 type in a pure in-kernel datastructure look off.
 
 v2: Ville asked for an overflow check since no one prevents userspace
 from incrementing the pin count forever.

UINT_MAX? You just changed the type to unsigned long.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Finish enabling rps before use by sysfs or debugfs

2013-10-10 Thread Daniel Vetter
On Tue, Sep 17, 2013 at 06:05:27PM +0200, Daniel Vetter wrote:
 On Tue, Sep 17, 2013 at 12:12 AM, Daniel Vetter dan...@ffwll.ch wrote:
  On Mon, Sep 16, 2013 at 02:56:43PM -0700, Tom.O'rou...@intel.com wrote:
  From: Tom O'Rourke Tom.O'rou...@intel.com
 
  Enabling rps (turbo setup) was put in a work queue because it may
  take quite awhile.  This change flushes the work queue to initialize
  rps values before use by sysfs or debugfs.  Specifically,
  rps.delayed_resume_work is flushed before using rps.hw_max,
  rps.max_delay, rps.min_delay, or rps.cur_delay.
 
  This change fixes a problem in sysfs where show functions using
  uninitialized values show incorrect values and store functions
  using uninitialized values in range checks incorrectly fail to
  store valid input values.  This change also addresses similar use
  before initialized problems in debugfs.
 
  Change-Id: Ib9c4f2066b65013094cb9278fc17958a964836e7
  Signed-off-by: Tom O'Rourke Tom.O'rou...@intel.com
 
  I think we can exercise this by going through a suspend/resume cycle and
  racing concurrent reads/writes of the sysfs files in a 2nd process. With
  the igt_fork and igt_system_suspend_autoresume helpers in our kernel
  testsuite repro this should be a really quick testcase to write ...
 
  I'd go for a generic read all sysfs files approach to increase the
  chances of catching other similar fallout in the future.
 
 To clarify: I'd like to see a testcase for this so that we can make
 sure it keeps working. I guess we unfortunately can't test for
 functional correctness by just resuming since we won't see
 uninitialized data. But at least we can test for the
 flush_delayed_work deadlock implications, which have bitten us badly
 in the past on numerous occasions. And maybe we can provoke hangs if
 we adjust the rps values while the setup hasn't completed yet, so
 might also give us weak coverage there.

I've stitched something together with little enthusiasm ...

*insert maintainer rant*

Patch merged.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: tell the user KMS is required for gen6+

2013-10-10 Thread Jani Nikula
Educate the users why i915 won't load on gen6+ and nomodeset.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=61671
Signed-off-by: Jani Nikula jani.nik...@intel.com
---
 drivers/gpu/drm/i915/i915_dma.c |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 0c86c48..9a54241 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1477,8 +1477,11 @@ int i915_driver_load(struct drm_device *dev, unsigned 
long flags)
info = (struct intel_device_info *) flags;
 
/* Refuse to load on gen6+ without kms enabled. */
-   if (info-gen = 6  !drm_core_check_feature(dev, DRIVER_MODESET))
+   if (info-gen = 6  !drm_core_check_feature(dev, DRIVER_MODESET)) {
+   DRM_INFO(Your hardware requires kernel modesetting (KMS)\n);
+   DRM_INFO(See CONFIG_DRM_I915_KMS, nomodeset, and i915.modeset 
parameters\n);
return -ENODEV;
+   }
 
dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
if (dev_priv == NULL)
-- 
1.7.10.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Use unsigned long for obj-user_pin_count

2013-10-10 Thread Daniel Vetter
At least on linux sizeof(long) == sizeof(void*) and the thinking
is that you can grab about as many references as there's memory.

Doesn't really matter, just a bit of OCD since the fixed size data
type in a pure in-kernel datastructure look off.

v2: Ville asked for an overflow check since no one prevents userspace
from incrementing the pin count forever.

v3: s/INT/LONG/, noticed by Chris.

Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_drv.h | 2 +-
 drivers/gpu/drm/i915/i915_gem.c | 5 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7fa017b..f39a6b8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1567,7 +1567,7 @@ struct drm_i915_gem_object {
unsigned long *bit_17;
 
/** User space pin count and filp owning the pin */
-   uint32_t user_pin_count;
+   unsigned long user_pin_count;
struct drm_file *pin_filp;
 
/** for phy allocated objects */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 71dd030..6988f81 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3929,6 +3929,11 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data,
goto out;
}
 
+   if (obj-user_pin_count == ULONG_MAX) {
+   ret = -EBUSY;
+   goto out;
+   }
+
if (obj-user_pin_count == 0) {
ret = i915_gem_obj_ggtt_pin(obj, args-alignment, true, false);
if (ret)
-- 
1.8.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v4 3/4] ACPI / video: Do not register backlight if win8 and native interface exists

2013-10-10 Thread Rafael J. Wysocki
On Thursday, October 10, 2013 09:02:55 AM Aaron Lu wrote:
 On 10/10/2013 08:29 AM, Rafael J. Wysocki wrote:
  On Tuesday, October 08, 2013 02:40:00 PM Aaron Lu wrote:
  According to Matthew Garrett, Windows 8 leaves backlight control up
  to individual graphics drivers rather than making ACPI calls itself.
  There's plenty of evidence to suggest that the Intel driver for
  Windows [8] doesn't use the ACPI interface, including the fact that
  it's broken on a bunch of machines when the OS claims to support
  Windows 8.  The simplest thing to do appears to be to disable the
  ACPI backlight interface on these systems.
 
  So for Win8 systems, if there is native backlight control interface
  registered by GPU driver, ACPI video will not register its own. For
  users who prefer to keep ACPI video's backlight interface, the existing
  kernel cmdline option acpi_backlight=video can be used.
 
  Signed-off-by: Aaron Lu aaron...@intel.com
  Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com
  Tested-by: Yves-Alexis Perez cor...@debian.org
  Tested-by: Mika Westerberg mika.westerb...@linux.intel.com
  ---
   drivers/acpi/internal.h |  5 ++---
   drivers/acpi/video.c| 10 +-
   drivers/acpi/video_detect.c | 14 --
   3 files changed, 19 insertions(+), 10 deletions(-)
 
  diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
  index 20f4233..453ae8d 100644
  --- a/drivers/acpi/internal.h
  +++ b/drivers/acpi/internal.h
  @@ -169,9 +169,8 @@ int acpi_create_platform_device(struct acpi_device 
  *adev,
 Video
 
  -- 
  */
   #if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
  -bool acpi_video_backlight_quirks(void);
  -#else
  -static inline bool acpi_video_backlight_quirks(void) { return false; }
  +bool acpi_osi_is_win8(void);
  +bool acpi_video_verify_backlight_support(void);
   #endif
   
   #endif /* _ACPI_INTERNAL_H_ */
  diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
  index 3bd1eaa..343db59 100644
  --- a/drivers/acpi/video.c
  +++ b/drivers/acpi/video.c
  @@ -1256,8 +1256,8 @@ acpi_video_switch_brightness(struct 
  acpi_video_device *device, int event)
 unsigned long long level_current, level_next;
 int result = -EINVAL;
   
  -  /* no warning message if acpi_backlight=vendor is used */
  -  if (!acpi_video_backlight_support())
  +  /* no warning message if acpi_backlight=vendor or a quirk is used */
  +  if (!acpi_video_verify_backlight_support())
 return 0;
   
 if (!device-brightness)
  @@ -1386,13 +1386,13 @@ acpi_video_bus_get_devices(struct acpi_video_bus 
  *video,
   static int acpi_video_bus_start_devices(struct acpi_video_bus *video)
   {
 return acpi_video_bus_DOS(video, 0,
  -acpi_video_backlight_quirks() ? 1 : 0);
  +acpi_osi_is_win8() ? 1 : 0);
   }
   
   static int acpi_video_bus_stop_devices(struct acpi_video_bus *video)
   {
 return acpi_video_bus_DOS(video, 0,
  -acpi_video_backlight_quirks() ? 0 : 1);
  +acpi_osi_is_win8() ? 0 : 1);
   }
   
   static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
  @@ -1558,7 +1558,7 @@ acpi_video_bus_match(acpi_handle handle, u32 level, 
  void *context,
   
   static void acpi_video_dev_register_backlight(struct acpi_video_device 
  *device)
   {
  -  if (acpi_video_backlight_support()) {
  +  if (acpi_video_verify_backlight_support()) {
 struct backlight_properties props;
 struct pci_dev *pdev;
 acpi_handle acpi_parent;
  diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
  index 940edbf..23d7d26 100644
  --- a/drivers/acpi/video_detect.c
  +++ b/drivers/acpi/video_detect.c
  @@ -37,6 +37,7 @@
   #include linux/acpi.h
   #include linux/dmi.h
   #include linux/pci.h
  +#include linux/backlight.h
   
   #include internal.h
   
  @@ -233,11 +234,11 @@ static void acpi_video_caps_check(void)
 acpi_video_get_capabilities(NULL);
   }
   
  -bool acpi_video_backlight_quirks(void)
  +bool acpi_osi_is_win8(void)
   {
 return acpi_gbl_osi_data = ACPI_OSI_WIN_8;
   }
  -EXPORT_SYMBOL(acpi_video_backlight_quirks);
  +EXPORT_SYMBOL(acpi_osi_is_win8);
   
   /* Promote the vendor interface instead of the generic video module.
* This function allow DMI blacklists to be implemented by externals
  @@ -283,6 +284,15 @@ int acpi_video_backlight_support(void)
   }
   EXPORT_SYMBOL(acpi_video_backlight_support);
   
  +bool acpi_video_verify_backlight_support(void)
  +{
  +  if (!(acpi_video_support  ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO) 
  +  acpi_osi_is_win8()  backlight_device_registered(BACKLIGHT_RAW))
  +  return false;
  
  If I'm not mistaken, this will introduce a regression for the people who 
  have
  problems with the native i915 backlight on 

Re: [Intel-gfx] [PATCH] drm/i915: Use unsigned long for obj-user_pin_count

2013-10-10 Thread Chris Wilson
On Thu, Oct 10, 2013 at 02:46:37PM +0200, Daniel Vetter wrote:
 At least on linux sizeof(long) == sizeof(void*) and the thinking
 is that you can grab about as many references as there's memory.
 
 Doesn't really matter, just a bit of OCD since the fixed size data
 type in a pure in-kernel datastructure look off.
 
 v2: Ville asked for an overflow check since no one prevents userspace
 from incrementing the pin count forever.
 
 v3: s/INT/LONG/, noticed by Chris.
 
 Cc: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Ville Syrjälä ville.syrj...@linux.intel.com
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 2/2] drm/i915: Do not enable package C8 on unsupported hardware

2013-10-10 Thread Chris Wilson
If the hardware does not support package C8, then do not even schedule
work to enable it. Thereby we can eliminate a bunch of dangerous work.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Paulo Zanoni paulo.r.zan...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h  |1 +
 drivers/gpu/drm/i915/intel_display.c |   15 +++
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7ba49d1..640bff2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1750,6 +1750,7 @@ struct drm_i915_file_private {
 #define HAS_POWER_WELL(dev)(IS_HASWELL(dev))
 #define HAS_FPGA_DBG_UNCLAIMED(dev)(INTEL_INFO(dev)-has_fpga_dbg)
 #define HAS_PSR(dev)   (IS_HASWELL(dev))
+#define HAS_PC8(dev)   (IS_HASWELL(dev)) /* XXX HSW:ULX */
 
 #define INTEL_PCH_DEVICE_ID_MASK   0xff00
 #define INTEL_PCH_IBX_DEVICE_ID_TYPE   0x3b00
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 4fa1fd5..2e9d75d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6479,6 +6479,9 @@ static void __hsw_disable_package_c8(struct 
drm_i915_private *dev_priv)
 
 void hsw_enable_package_c8(struct drm_i915_private *dev_priv)
 {
+   if (!HAS_PC8(dev_priv-dev))
+   return;
+
mutex_lock(dev_priv-pc8.lock);
__hsw_enable_package_c8(dev_priv);
mutex_unlock(dev_priv-pc8.lock);
@@ -6486,6 +6489,9 @@ void hsw_enable_package_c8(struct drm_i915_private 
*dev_priv)
 
 void hsw_disable_package_c8(struct drm_i915_private *dev_priv)
 {
+   if (!HAS_PC8(dev_priv-dev))
+   return;
+
mutex_lock(dev_priv-pc8.lock);
__hsw_disable_package_c8(dev_priv);
mutex_unlock(dev_priv-pc8.lock);
@@ -6523,6 +6529,9 @@ static void hsw_update_package_c8(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev-dev_private;
bool allow;
 
+   if (!HAS_PC8(dev_priv-dev))
+   return;
+
if (!i915_enable_pc8)
return;
 
@@ -6546,6 +6555,9 @@ done:
 
 static void hsw_package_c8_gpu_idle(struct drm_i915_private *dev_priv)
 {
+   if (!HAS_PC8(dev_priv-dev))
+   return;
+
mutex_lock(dev_priv-pc8.lock);
if (!dev_priv-pc8.gpu_idle) {
dev_priv-pc8.gpu_idle = true;
@@ -6556,6 +6568,9 @@ static void hsw_package_c8_gpu_idle(struct 
drm_i915_private *dev_priv)
 
 static void hsw_package_c8_gpu_busy(struct drm_i915_private *dev_priv)
 {
+   if (!HAS_PC8(dev_priv-dev))
+   return;
+
mutex_lock(dev_priv-pc8.lock);
if (dev_priv-pc8.gpu_idle) {
dev_priv-pc8.gpu_idle = false;
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 1/2] drm/i915: Hold pc8 lock around toggling pc8.gpu_idle

2013-10-10 Thread Chris Wilson
We need to hold the pc8 lock around toggling the value of gpu_idle.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Paulo Zanoni paulo.r.zan...@intel.com
---
 drivers/gpu/drm/i915/intel_display.c |8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 5ce1558..4fa1fd5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6546,18 +6546,22 @@ done:
 
 static void hsw_package_c8_gpu_idle(struct drm_i915_private *dev_priv)
 {
+   mutex_lock(dev_priv-pc8.lock);
if (!dev_priv-pc8.gpu_idle) {
dev_priv-pc8.gpu_idle = true;
-   hsw_enable_package_c8(dev_priv);
+   __hsw_enable_package_c8(dev_priv);
}
+   mutex_unlock(dev_priv-pc8.lock);
 }
 
 static void hsw_package_c8_gpu_busy(struct drm_i915_private *dev_priv)
 {
+   mutex_lock(dev_priv-pc8.lock);
if (dev_priv-pc8.gpu_idle) {
dev_priv-pc8.gpu_idle = false;
-   hsw_disable_package_c8(dev_priv);
+   __hsw_disable_package_c8(dev_priv);
}
+   mutex_unlock(dev_priv-pc8.lock);
 }
 
 static void haswell_modeset_global_resources(struct drm_device *dev)
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm: add support for additional stereo 3D modes

2013-10-10 Thread Thomas Wood
Parse the 3D_Structure_ALL and 3D_MASK fields of the HDMI Vendor
Specific Data Block to expose more stereo 3D modes.

Signed-off-by: Thomas Wood thomas.w...@intel.com
---
 drivers/gpu/drm/drm_edid.c | 93 ++
 1 file changed, 85 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9e81609..b3949f9 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2652,6 +2652,50 @@ static int add_hdmi_mode(struct drm_connector 
*connector, u8 vic)
return 1;
 }
 
+static int add_3d_struct_modes(struct drm_connector *connector, u16 structure,
+  const u8 *video_db, u8 video_len, u8 video_index)
+{
+   struct drm_device *dev = connector-dev;
+   struct drm_display_mode *newmode;
+   int modes = 0;
+   u8 cea_mode;
+
+   if (video_db == NULL || video_index  video_len)
+   return 0;
+
+   /* CEA modes are numbered 1..127 */
+   cea_mode = (video_db[video_index]  127) - 1;
+   if (cea_mode = ARRAY_SIZE(edid_cea_modes))
+   return 0;
+
+   if (structure  1) {
+   newmode = drm_mode_duplicate(dev, edid_cea_modes[cea_mode]);
+   if (newmode) {
+   newmode-flags |= DRM_MODE_FLAG_3D_FRAME_PACKING;
+   drm_mode_probed_add(connector, newmode);
+   modes++;
+   }
+   }
+   if (structure  (1  6)) {
+   newmode = drm_mode_duplicate(dev, edid_cea_modes[cea_mode]);
+   if (newmode) {
+   newmode-flags |= DRM_MODE_FLAG_3D_TOP_AND_BOTTOM;
+   drm_mode_probed_add(connector, newmode);
+   modes++;
+   }
+   }
+   if (structure  (1  8)) {
+   newmode = drm_mode_duplicate(dev, edid_cea_modes[cea_mode]);
+   if (newmode) {
+   newmode-flags = DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF;
+   drm_mode_probed_add(connector, newmode);
+   modes++;
+   }
+   }
+
+   return modes;
+}
+
 /*
  * do_hdmi_vsdb_modes - Parse the HDMI Vendor Specific data block
  * @connector: connector corresponding to the HDMI sink
@@ -2662,10 +2706,13 @@ static int add_hdmi_mode(struct drm_connector 
*connector, u8 vic)
  * also adds the stereo 3d modes when applicable.
  */
 static int
-do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len)
+do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len,
+  const u8 *video_db, u8 video_len)
 {
-   int modes = 0, offset = 0, i;
+   int modes = 0, offset = 0, i, multi_present = 0;
u8 vic_len;
+   u16 mask;
+   u16 structure_all;
 
if (len  8)
goto out;
@@ -2689,9 +2736,13 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, 
const u8 *db, u8 len)
 
/* 3D_Present */
offset++;
-   if (db[8 + offset]  (1  7))
+   if (db[8 + offset]  (1  7)) {
modes += add_hdmi_mandatory_stereo_modes(connector);
 
+   /* 3D_Multi_present */
+   multi_present = (db[8 + offset]  0x60)  5;
+   }
+
offset++;
vic_len = db[8 + offset]  5;
 
@@ -2702,6 +2753,28 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, 
const u8 *db, u8 len)
modes += add_hdmi_mode(connector, vic);
}
 
+   offset += 1 + vic_len;
+
+   if ((multi_present == 1 || multi_present == 2) 
+   len = (9 + offset)) {
+   /* 3D_Structure_ALL */
+   structure_all = (db[8 + offset]  8) | db[9 + offset];
+
+   /* check if 3D_MASK is present */
+   if (multi_present == 2  len = 11 + offset)
+   mask = (db[10 + offset]  8) | db[11 + offset];
+   else
+   mask = 0x;
+
+   for (i = 0; i  16; i++) {
+   if (mask  (1  i))
+   modes += add_3d_struct_modes(connector,
+structure_all,
+video_db,
+video_len, i);
+   }
+   }
+
 out:
return modes;
 }
@@ -2759,8 +2832,8 @@ static int
 add_cea_modes(struct drm_connector *connector, struct edid *edid)
 {
const u8 *cea = drm_find_cea_extension(edid);
-   const u8 *db, *hdmi = NULL;
-   u8 dbl, hdmi_len;
+   const u8 *db, *hdmi = NULL, *video = NULL;
+   u8 dbl, hdmi_len, video_len = 0;
int modes = 0;
 
if (cea  cea_revision(cea) = 3) {
@@ -2773,8 +2846,11 @@ add_cea_modes(struct drm_connector *connector, struct 
edid *edid)
db = cea[i];
dbl = cea_db_payload_len(db);
 
-   

Re: [Intel-gfx] [PATCH] drm/i915: tell the user KMS is required for gen6+

2013-10-10 Thread Ville Syrjälä
On Thu, Oct 10, 2013 at 03:25:37PM +0300, Jani Nikula wrote:
 Educate the users why i915 won't load on gen6+ and nomodeset.
 
 Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=61671
 Signed-off-by: Jani Nikula jani.nik...@intel.com

Yes, much better than the agp wtf error people get currently.

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

 ---
  drivers/gpu/drm/i915/i915_dma.c |5 -
  1 file changed, 4 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
 index 0c86c48..9a54241 100644
 --- a/drivers/gpu/drm/i915/i915_dma.c
 +++ b/drivers/gpu/drm/i915/i915_dma.c
 @@ -1477,8 +1477,11 @@ int i915_driver_load(struct drm_device *dev, unsigned 
 long flags)
   info = (struct intel_device_info *) flags;
  
   /* Refuse to load on gen6+ without kms enabled. */
 - if (info-gen = 6  !drm_core_check_feature(dev, DRIVER_MODESET))
 + if (info-gen = 6  !drm_core_check_feature(dev, DRIVER_MODESET)) {
 + DRM_INFO(Your hardware requires kernel modesetting (KMS)\n);
 + DRM_INFO(See CONFIG_DRM_I915_KMS, nomodeset, and i915.modeset 
 parameters\n);
   return -ENODEV;
 + }
  
   dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
   if (dev_priv == NULL)
 -- 
 1.7.10.4
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] Macmini6, 1 with i915 2:2.20.0-0ubuntu0~precise1 Hard Freezing and drm_ioctl Debug Message Help

2013-10-10 Thread Preston Connors
I am having an issue where my Macmini6,1 is hard freezing (no keyboard
control, have to power off and power back on using the power button) when
attempting to display a random video at random times but always freezing
when the video should be displayed by VLC during the start of the video. I
am suspecting it may be the i915 driver communicating with the Intel
Corporation 3rd Gen Core processor Graphics Controller (rev 09) graphics
controller. I'm having a problem debugging and pinpointing exactly what
might be freezing. and any help would be appreciated. This MacMini is
basically a simple kiosk that plays an mp4 video downloaded from Youtube;
the video plays, exits, and the next video plays ad infinitum.

The final message from vlc -vvv when the system freezes:
[0x7f91f0001248] xcb vout display debug: display is visible

With default kernel / device logging verbosities nothing is logged to
/var/log/syslog or /var/log/kern.log when the system freezes.

i915 package information:
Package: xserver-xorg-video-intel
Version: 2:2.20.0-0ubuntu0~precise1

kernel: 3.2.0-54-generic x86_64

vlc version: 2.2.0~~git20131009+r3076-0~r106+100~ubuntu12.04.1

I enabled some verbose debug options from grub to enable more verbose
output today:
rootwait ignore_loglevel debug debug_locks_verbose=1 sched_debug
initcall_debug mminit_loglevel=4 udev.log_priority=8 loglevel=8
earlyprintk=vga,keep log_buf_len=10M print_fatal_signals=1 apm.debug=Y
i8042.debug=Y drm.debug=1 scsi_logging_level=1 usbserial.debug=Y
option.debug=Y pl2303.debug=Y firewire_ohci.debug=1 hid.debug=1
pci_hotplug.debug=Y pci_hotplug.debug_acpi=Y shpchp.shpchp_debug=Y
apic=debug show_lapic=all hpet=verbose lmb=debug pause_on_oops=5 panic=10
sysrq_always_enabled

These debugging options enabled drm:drm_ioctl to create 576 messages per
second being written to /var/log/syslog and /var/log/kern.log. These
messages are being generated while the system is operating normally
displaying an mp4 video via VLC.

Can you please help me out with what these debug messages mean? Are they
benign?

Here is the condensed version of the messages from /var/log/kern.log:

msg_count, msg

240 [drm:drm_ioctl], pid=1416, cmd=0x4020645d, nr=0x5d, dev 0xe200,
auth=1
144 [drm:drm_ioctl], pid=1416, cmd=0xc00c6466, nr=0x66, dev 0xe200,
auth=1
 96 [drm:drm_ioctl], pid=1416, cmd=0xc0086457, nr=0x57, dev 0xe200,
auth=1
 24 [drm:drm_ioctl], pid=1416, cmd=0xc06864a1, nr=0xa1, dev 0xe200,
auth=1
 24 [drm:drm_ioctl], pid=1416, cmd=0x6458, nr=0x58, dev 0xe200, auth=1
 24 [drm:drm_ioctl], pid=1416, cmd=0x40406469, nr=0x69, dev 0xe200,
auth=1
 24 [drm:drm_ioctl], pid=1416, cmd=0x400c645f, nr=0x5f, dev 0xe200,
auth=1


Unabridged version of one second of messages:
http://pastebin.com/PxS9giiZ

Once I understand these messages I'm going to wait for the system to fail
and see if there are different or new debug messages. Thanks in advance for
your help! I'm willing to troubleshoot and try different things to
determine the root cause of this freezing.

-- 
Thank you,
Preston Connors
407-283-7806
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] linux-next: manual merge of the drm-intel tree

2013-10-10 Thread Mark Brown
Today's linux-next merge of the drm-intel tree got additional conflicts
in drivers/gpu/drm/i915/intel_drv.h as a result of interactions between
6aba5b6cf098 (drm/i915/dp: get rid of intel_dp-link_configuration) and
various commits from Paulo Zanoni staticising functions.  I've fixed up
by changing the resolution to that below and can carry:

diff --cc drivers/gpu/drm/i915/i915_dma.c
index 637b695,df6efbf..0c86c48
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@@ -1330,9 -1333,11 +1333,11 @@@ static int i915_load_modeset_init(struc
  
/* Always safe in the mode setting case. */
/* FIXME: do pre/post-mode set stuff in core KMS code */
 -  dev-vblank_disable_allowed = 1;
 +  dev-vblank_disable_allowed = true;
-   if (INTEL_INFO(dev)-num_pipes == 0)
+   if (INTEL_INFO(dev)-num_pipes == 0) {
+   intel_display_power_put(dev, POWER_DOMAIN_VGA);
return 0;
+   }
  
ret = intel_fbdev_init(dev);
if (ret)
@@@ -1473,7 -1480,14 +1480,7 @@@ int i915_driver_load(struct drm_device 
if (info-gen = 6  !drm_core_check_feature(dev, DRIVER_MODESET))
return -ENODEV;
  
-   dev_priv = kzalloc(sizeof(drm_i915_private_t), GFP_KERNEL);
 -  /* i915 has 4 more counters */
 -  dev-counters += 4;
 -  dev-types[6] = _DRM_STAT_IRQ;
 -  dev-types[7] = _DRM_STAT_PRIMARY;
 -  dev-types[8] = _DRM_STAT_SECONDARY;
 -  dev-types[9] = _DRM_STAT_DMA;
 -
+   dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
if (dev_priv == NULL)
return -ENOMEM;
  
diff --cc drivers/gpu/drm/i915/intel_dp.c
index 98f3b64,f831464..c392ad2
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@@ -1532,8 -1558,8 +1541,8 @@@ static void intel_edp_psr_setup(struct 
intel_edp_psr_write_vsc(intel_dp, psr_vsc);
  
/* Avoid continuous PSR exit by masking memup and hpd */
-   I915_WRITE(EDP_PSR_DEBUG_CTL, EDP_PSR_DEBUG_MASK_MEMUP |
+   I915_WRITE(EDP_PSR_DEBUG_CTL(dev), EDP_PSR_DEBUG_MASK_MEMUP |
 - EDP_PSR_DEBUG_MASK_HPD);
 + EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP);
  
intel_dp-psr_setup_done = true;
  }


signature.asc
Description: Digital signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: tell the user KMS is required for gen6+

2013-10-10 Thread Daniel Vetter
On Thu, Oct 10, 2013 at 05:28:54PM +0300, Ville Syrjälä wrote:
 On Thu, Oct 10, 2013 at 03:25:37PM +0300, Jani Nikula wrote:
  Educate the users why i915 won't load on gen6+ and nomodeset.
  
  Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=61671
  Signed-off-by: Jani Nikula jani.nik...@intel.com
 
 Yes, much better than the agp wtf error people get currently.
 
 Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Fix pipe off timeout handling for pre-gen4

2013-10-10 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

The current pre-gen4 pipe off code might break out of the loop
due to the timeout, but then the fail to print the warning.

Fix the issue by making sure we pair up the correct time comparison
functions. It would be enough to change just the final check to use
time_after_eq(), but also changing the loop condition to use
time_before() instead of time_after() makes the whole thing look a
bit saner since then we always compare jiffies and timeout in the same
order, and the words before and after now feel more natural when
talking about the timeout.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_display.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 27f98bc..07cd8f8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -845,8 +845,8 @@ void intel_wait_for_pipe_off(struct drm_device *dev, int 
pipe)
last_line = I915_READ(reg)  line_mask;
mdelay(5);
} while (((I915_READ(reg)  line_mask) != last_line) 
-time_after(timeout, jiffies));
-   if (time_after(jiffies, timeout))
+time_before(jiffies, timeout));
+   if (time_after_eq(jiffies, timeout))
WARN(1, pipe_off wait timed out\n);
}
 }
-- 
1.8.1.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 4/4] drm/i915: skip useless vblank wait on Haswell audio sequence

2013-10-10 Thread Paulo Zanoni
2013/9/22 Lin, Mengdong mengdong@intel.com:
 Hi Daniel and Paulo,

 I think we need a confirmation from HW owner whether vblank wait can be 
 skipped in audio enabling and disabling. I'll ping HW owner and involve you.
 The original code just follows b-spec but there is no explanation why.

Hi, any news on this?

Please notice that even though the spec says we need to wait for a
vblank, this code runs on a place where the pipe is disabled, so
there's no vblank to wait. Bspec also suggests Audio should only be
enabled at the very end of the mode set sequence, so the proper fix
would probably be to move the place where haswell_write_eld is called.
While this is not really done, I think we could probably merge this
patch because the 50ms delay here seems pointless.

Thanks,
Paulo


 Thanks
 Mengdong

 -Original Message-
 From: intel-gfx-bounces+mengdong.lin=intel@lists.freedesktop.org
 [mailto:intel-gfx-bounces+mengdong.lin=intel@lists.freedesktop.org] On
 Behalf Of Daniel Vetter
 Sent: Friday, September 20, 2013 4:18 PM
 To: Paulo Zanoni
 Cc: intel-gfx@lists.freedesktop.org; Zanoni, Paulo R
 Subject: Re: [Intel-gfx] [PATCH 4/4] drm/i915: skip useless vblank wait on
 Haswell audio sequence

 On Thu, Sep 19, 2013 at 05:07:29PM -0300, Paulo Zanoni wrote:
  From: Paulo Zanoni paulo.r.zan...@intel.com
 
  We call haswell_write_eld at mode_set time, not at crtc_enable time,
  so the pipes are stopped, and it doesn't really make sense to wait for
  a vblank: it just delays the modeset in 50ms.
 
  Leave the code there (commented with FIXME) for now since maybe we
  need a bigger rework.
 
  Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
  ---
   drivers/gpu/drm/i915/intel_display.c | 14 +++---
   1 file changed, 11 insertions(+), 3 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/intel_display.c
  b/drivers/gpu/drm/i915/intel_display.c
  index 69e8bb6..b891a0c 100644
  --- a/drivers/gpu/drm/i915/intel_display.c
  +++ b/drivers/gpu/drm/i915/intel_display.c
  @@ -6662,7 +6662,6 @@ static void haswell_write_eld(struct
  drm_connector *connector,  {
  struct drm_i915_private *dev_priv = connector-dev-dev_private;
  uint8_t *eld = connector-eld;
  -   struct drm_device *dev = crtc-dev;
  struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
  uint32_t eldv;
  uint32_t i;
  @@ -6684,8 +6683,17 @@ static void haswell_write_eld(struct
 drm_connector *connector,
  tmp |= (AUDIO_OUTPUT_ENABLE_A  (pipe * 4));
  I915_WRITE(aud_cntrl_st2, tmp);
 
  -   /* Wait for 1 vertical blank */
  -   intel_wait_for_vblank(dev, pipe);
  +   /*
  +* Wait for 1 vertical blank
  +*
  +* FIXME: We call this function at mode_set time, when the pipes are 
  all
  +* stopped, so it doesn't really make any sense to wait for a vblank
  +* here: it just delays the modeset in 50ms. I'll leave the code here
  +* because since the wait doesn't make sense at this point, maybe we
  +* need a bigger rework. We need an Audio authority to audit this code.
  +*
  +* intel_wait_for_vblank(dev_priv-dev, pipe);
  +*/

 This might be due to the generic recommendation that infoframes and related
 stuff (audio also gets transmitted when infoframes are) should only be 
 changed
 after the vblank completed when the pipe is on.

 Imo we should just ditch this (and cc: the audio guys on the patch so they're
 aware).
 -Daniel

 
  /* Set ELD valid state */
  tmp = I915_READ(aud_cntrl_st2);
  --
  1.8.3.1
 
  ___
  Intel-gfx mailing list
  Intel-gfx@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/intel-gfx

 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 4/4] drm/i915: skip useless vblank wait on Haswell audio sequence

2013-10-10 Thread Daniel Vetter
On Thu, Oct 10, 2013 at 8:33 PM, Paulo Zanoni przan...@gmail.com wrote:
 2013/9/22 Lin, Mengdong mengdong@intel.com:
 Hi Daniel and Paulo,

 I think we need a confirmation from HW owner whether vblank wait can be 
 skipped in audio enabling and disabling. I'll ping HW owner and involve you.
 The original code just follows b-spec but there is no explanation why.

 Hi, any news on this?

 Please notice that even though the spec says we need to wait for a
 vblank, this code runs on a place where the pipe is disabled, so
 there's no vblank to wait. Bspec also suggests Audio should only be
 enabled at the very end of the mode set sequence, so the proper fix
 would probably be to move the place where haswell_write_eld is called.
 While this is not really done, I think we could probably merge this
 patch because the 50ms delay here seems pointless.

I think updating the ELD is one of those things we can do any time, as
long as it's early enough (similar to setting new pipe timings). Maybe
we need to move the updating of the eld_valid bits (which is what will
generate the unsolictid interrupt event on the audio driver side
afaik). But we have a sparate bit (plus interrupt type on the audio
side) for audio is now enabled and I think that's the really
important part. And that one is enabled at the right spot in the
modeset sequence afaics.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Hold pc8 lock around toggling pc8.gpu_idle

2013-10-10 Thread Paulo Zanoni
2013/10/10 Chris Wilson ch...@chris-wilson.co.uk:
 We need to hold the pc8 lock around toggling the value of gpu_idle.

 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Paulo Zanoni paulo.r.zan...@intel.com

Should we Cc:stable ?

Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com

I wonder if we need to rename hsw_enable_package_c8 and
__hsw_enable_package_c8 since we're spreading the usage of the __
function. Suggestions/patches welcome :)

 ---
  drivers/gpu/drm/i915/intel_display.c |8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 5ce1558..4fa1fd5 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -6546,18 +6546,22 @@ done:

  static void hsw_package_c8_gpu_idle(struct drm_i915_private *dev_priv)
  {
 +   mutex_lock(dev_priv-pc8.lock);
 if (!dev_priv-pc8.gpu_idle) {
 dev_priv-pc8.gpu_idle = true;
 -   hsw_enable_package_c8(dev_priv);
 +   __hsw_enable_package_c8(dev_priv);
 }
 +   mutex_unlock(dev_priv-pc8.lock);
  }

  static void hsw_package_c8_gpu_busy(struct drm_i915_private *dev_priv)
  {
 +   mutex_lock(dev_priv-pc8.lock);
 if (dev_priv-pc8.gpu_idle) {
 dev_priv-pc8.gpu_idle = false;
 -   hsw_disable_package_c8(dev_priv);
 +   __hsw_disable_package_c8(dev_priv);
 }
 +   mutex_unlock(dev_priv-pc8.lock);
  }

  static void haswell_modeset_global_resources(struct drm_device *dev)
 --
 1.7.9.5

 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Cancel outstanding pc8 work when shutting down the device

2013-10-10 Thread Paulo Zanoni
2013/10/10 Chris Wilson ch...@chris-wilson.co.uk:
 As part of the device quiesceing we need to disable all active timers
 and delayed workers so that they do not execute after the module is
 unloaded.

 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Paulo Zanoni paulo.r.zan...@intel.com
 Cc: Rodrigo Vivi rodrigo.v...@gmail.com
 ---
  drivers/gpu/drm/i915/intel_display.c |1 +
  1 file changed, 1 insertion(+)

 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 683b68e..5ce1558 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -11065,6 +11065,7 @@ void intel_modeset_quiesce(struct drm_device *dev)

I don't see this intel_modeset_quiesce on my drm-intel-nightly
branch from today :(

Besides, we already cancel the work at i915_driver_unload().


 cancel_work_sync(dev_priv-hotplug_work);
 cancel_work_sync(dev_priv-rps.work);
 +   cancel_delayed_work_sync(dev_priv-pc8.enable_work);

 /* catch all required for dev_priv-wq */
 flush_scheduled_work();
 --
 1.7.9.5

 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/2] drm/i915: Do not enable package C8 on unsupported hardware

2013-10-10 Thread Paulo Zanoni
2013/10/10 Chris Wilson ch...@chris-wilson.co.uk:
 If the hardware does not support package C8, then do not even schedule
 work to enable it. Thereby we can eliminate a bunch of dangerous work.

As I already explained, this should not be a problem since non-Haswell
platforms don't have a way to make the refcount become zero (unless we
have a bug). I also asked people's opinions about this specific
decision in one of my cover letters, but no one said anything:
http://lists.freedesktop.org/archives/intel-gfx/2013-August/031440.html.

Quoting the email: Another thing worth mentioning is that all this
code doesn't have IS_HASWELL checks, and on non-Haswell platforms the
refcount will never reach 0, so we won't ever try to enable PC8. I'm
not sure if that's what we want, so please comment on that..

That said, I'm not against your changes.

But for completeness, you should probably add a WARN(!HAS_PC8()) at
haswell_enable_pc8_work().


 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Paulo Zanoni paulo.r.zan...@intel.com
 ---
  drivers/gpu/drm/i915/i915_drv.h  |1 +
  drivers/gpu/drm/i915/intel_display.c |   15 +++
  2 files changed, 16 insertions(+)

 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 7ba49d1..640bff2 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -1750,6 +1750,7 @@ struct drm_i915_file_private {
  #define HAS_POWER_WELL(dev)(IS_HASWELL(dev))
  #define HAS_FPGA_DBG_UNCLAIMED(dev)(INTEL_INFO(dev)-has_fpga_dbg)
  #define HAS_PSR(dev)   (IS_HASWELL(dev))
 +#define HAS_PC8(dev)   (IS_HASWELL(dev)) /* XXX HSW:ULX */

What exactly do you mean with this comment? Did you actually mean
IS_ULT()? Even though only ULT has PC8-10 residencies, non-ULT seems
to work fine with this code, so I thought it wouldn't be a problem.



  #define INTEL_PCH_DEVICE_ID_MASK   0xff00
  #define INTEL_PCH_IBX_DEVICE_ID_TYPE   0x3b00
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 4fa1fd5..2e9d75d 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -6479,6 +6479,9 @@ static void __hsw_disable_package_c8(struct 
 drm_i915_private *dev_priv)

  void hsw_enable_package_c8(struct drm_i915_private *dev_priv)
  {
 +   if (!HAS_PC8(dev_priv-dev))
 +   return;
 +
 mutex_lock(dev_priv-pc8.lock);
 __hsw_enable_package_c8(dev_priv);
 mutex_unlock(dev_priv-pc8.lock);
 @@ -6486,6 +6489,9 @@ void hsw_enable_package_c8(struct drm_i915_private 
 *dev_priv)

  void hsw_disable_package_c8(struct drm_i915_private *dev_priv)
  {
 +   if (!HAS_PC8(dev_priv-dev))
 +   return;
 +
 mutex_lock(dev_priv-pc8.lock);
 __hsw_disable_package_c8(dev_priv);
 mutex_unlock(dev_priv-pc8.lock);
 @@ -6523,6 +6529,9 @@ static void hsw_update_package_c8(struct drm_device 
 *dev)
 struct drm_i915_private *dev_priv = dev-dev_private;
 bool allow;

 +   if (!HAS_PC8(dev_priv-dev))
 +   return;
 +
 if (!i915_enable_pc8)
 return;

 @@ -6546,6 +6555,9 @@ done:

  static void hsw_package_c8_gpu_idle(struct drm_i915_private *dev_priv)
  {
 +   if (!HAS_PC8(dev_priv-dev))
 +   return;
 +
 mutex_lock(dev_priv-pc8.lock);
 if (!dev_priv-pc8.gpu_idle) {
 dev_priv-pc8.gpu_idle = true;
 @@ -6556,6 +6568,9 @@ static void hsw_package_c8_gpu_idle(struct 
 drm_i915_private *dev_priv)

  static void hsw_package_c8_gpu_busy(struct drm_i915_private *dev_priv)
  {
 +   if (!HAS_PC8(dev_priv-dev))
 +   return;
 +
 mutex_lock(dev_priv-pc8.lock);
 if (dev_priv-pc8.gpu_idle) {
 dev_priv-pc8.gpu_idle = false;
 --
 1.7.9.5

 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/2] drm/i915: Do not enable package C8 on unsupported hardware

2013-10-10 Thread Chris Wilson
On Thu, Oct 10, 2013 at 05:17:31PM -0300, Paulo Zanoni wrote:
 2013/10/10 Chris Wilson ch...@chris-wilson.co.uk:
  If the hardware does not support package C8, then do not even schedule
  work to enable it. Thereby we can eliminate a bunch of dangerous work.
 
 As I already explained, this should not be a problem since non-Haswell
 platforms don't have a way to make the refcount become zero (unless we
 have a bug). I also asked people's opinions about this specific
 decision in one of my cover letters, but no one said anything:
 http://lists.freedesktop.org/archives/intel-gfx/2013-August/031440.html.
 
 Quoting the email: Another thing worth mentioning is that all this
 code doesn't have IS_HASWELL checks, and on non-Haswell platforms the
 refcount will never reach 0, so we won't ever try to enable PC8. I'm
 not sure if that's what we want, so please comment on that..
 
 That said, I'm not against your changes.

If they don't actually fix anything, they are low priority as they only
remove a mutex lock at most 10Hz. Maybe a comment would be good to remind
the next person that nothing gets enabled except on hsw.

  +#define HAS_PC8(dev)   (IS_HASWELL(dev)) /* XXX HSW:ULX */
 
 What exactly do you mean with this comment? Did you actually mean
 IS_ULT()? Even though only ULT has PC8-10 residencies, non-ULT seems
 to work fine with this code, so I thought it wouldn't be a problem.

It means I didn't actually check the valid restrictions :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Fix pipe off timeout handling for pre-gen4

2013-10-10 Thread Daniel Vetter
On Thu, Oct 10, 2013 at 08:32:23PM +0300, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä ville.syrj...@linux.intel.com
 
 The current pre-gen4 pipe off code might break out of the loop
 due to the timeout, but then the fail to print the warning.
 
 Fix the issue by making sure we pair up the correct time comparison
 functions. It would be enough to change just the final check to use
 time_after_eq(), but also changing the loop condition to use
 time_before() instead of time_after() makes the whole thing look a
 bit saner since then we always compare jiffies and timeout in the same
 order, and the words before and after now feel more natural when
 talking about the timeout.
 
 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Hold pc8 lock around toggling pc8.gpu_idle

2013-10-10 Thread Chris Wilson
On Thu, Oct 10, 2013 at 05:04:27PM -0300, Paulo Zanoni wrote:
 2013/10/10 Chris Wilson ch...@chris-wilson.co.uk:
  We need to hold the pc8 lock around toggling the value of gpu_idle.
 
  Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
  Cc: Paulo Zanoni paulo.r.zan...@intel.com
 
 Should we Cc:stable ?

Perhaps. Maybe this is a race condition that results in the enable_work
being kicked off...
 
 Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
 
 I wonder if we need to rename hsw_enable_package_c8 and
 __hsw_enable_package_c8 since we're spreading the usage of the __
 function. Suggestions/patches welcome :)

__hsw_enable_package_c8 follows the idiom of being the inner locked
variant that one should only call if they know they meet the
preconditions. It keeps the longer name as a deterrent against use.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Cancel outstanding pc8 work when shutting down the device

2013-10-10 Thread Chris Wilson
On Thu, Oct 10, 2013 at 05:21:37PM -0300, Paulo Zanoni wrote:
 2013/10/10 Chris Wilson ch...@chris-wilson.co.uk:
  As part of the device quiesceing we need to disable all active timers
  and delayed workers so that they do not execute after the module is
  unloaded.
 
  Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
  Cc: Paulo Zanoni paulo.r.zan...@intel.com
  Cc: Rodrigo Vivi rodrigo.v...@gmail.com
  ---
   drivers/gpu/drm/i915/intel_display.c |1 +
   1 file changed, 1 insertion(+)
 
  diff --git a/drivers/gpu/drm/i915/intel_display.c 
  b/drivers/gpu/drm/i915/intel_display.c
  index 683b68e..5ce1558 100644
  --- a/drivers/gpu/drm/i915/intel_display.c
  +++ b/drivers/gpu/drm/i915/intel_display.c
  @@ -11065,6 +11065,7 @@ void intel_modeset_quiesce(struct drm_device *dev)
 
 I don't see this intel_modeset_quiesce on my drm-intel-nightly
 branch from today :(
 
 Besides, we already cancel the work at i915_driver_unload().

Well that at least allays my immediate fears that the timer is still
running then. However, it probably wants to be hsw_disable_package_c8()
instead.

Oh well, we know a timer is still running somewhere.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Fix pipe off timeout handling for pre-gen4

2013-10-10 Thread Chris Wilson
On Thu, Oct 10, 2013 at 08:32:23PM +0300, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä ville.syrj...@linux.intel.com
 
 The current pre-gen4 pipe off code might break out of the loop
 due to the timeout, but then the fail to print the warning.
 
 Fix the issue by making sure we pair up the correct time comparison
 functions. It would be enough to change just the final check to use
 time_after_eq(), but also changing the loop condition to use
 time_before() instead of time_after() makes the whole thing look a
 bit saner since then we always compare jiffies and timeout in the same
 order, and the words before and after now feel more natural when
 talking about the timeout.

Don't you also want to recheck last_line just in case it managed to
finish in the nick of time?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Avoid tweaking RPS before it is enabled

2013-10-10 Thread Chris Wilson
As we delay the initial RPS enabling (upon boot and after resume), there
is a chance that we may start to render and trigger RPS boosts before we
set up the punit. Any changes we make could result in inconsistent
hardware state, with a danger of causing undefined behaviour. However,
as the boosting is a optional tweak to RPS, we can simply ignore it
whilst RPS is not yet enabled.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Jesse Barnes jbar...@virtuousgeek.org
---
 drivers/gpu/drm/i915/i915_drv.h |1 +
 drivers/gpu/drm/i915/intel_pm.c |   26 --
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 640bff2..e0152e7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -874,6 +874,7 @@ struct intel_gen6_power_mgmt {
int last_adj;
enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
 
+   bool enabled;
struct delayed_work delayed_resume_work;
 
/*
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6ffeb04..8070a07 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3435,22 +3435,26 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
 void gen6_rps_idle(struct drm_i915_private *dev_priv)
 {
mutex_lock(dev_priv-rps.hw_lock);
-   if (dev_priv-info-is_valleyview)
-   valleyview_set_rps(dev_priv-dev, dev_priv-rps.min_delay);
-   else
-   gen6_set_rps(dev_priv-dev, dev_priv-rps.min_delay);
-   dev_priv-rps.last_adj = 0;
+   if (dev_priv-rps.enabled) {
+   if (dev_priv-info-is_valleyview)
+   valleyview_set_rps(dev_priv-dev, 
dev_priv-rps.min_delay);
+   else
+   gen6_set_rps(dev_priv-dev, dev_priv-rps.min_delay);
+   dev_priv-rps.last_adj = 0;
+   }
mutex_unlock(dev_priv-rps.hw_lock);
 }
 
 void gen6_rps_boost(struct drm_i915_private *dev_priv)
 {
mutex_lock(dev_priv-rps.hw_lock);
-   if (dev_priv-info-is_valleyview)
-   valleyview_set_rps(dev_priv-dev, dev_priv-rps.max_delay);
-   else
-   gen6_set_rps(dev_priv-dev, dev_priv-rps.max_delay);
-   dev_priv-rps.last_adj = 0;
+   if (dev_priv-rps.enabled) {
+   if (dev_priv-info-is_valleyview)
+   valleyview_set_rps(dev_priv-dev, 
dev_priv-rps.max_delay);
+   else
+   gen6_set_rps(dev_priv-dev, dev_priv-rps.max_delay);
+   dev_priv-rps.last_adj = 0;
+   }
mutex_unlock(dev_priv-rps.hw_lock);
 }
 
@@ -4657,6 +4661,7 @@ void intel_disable_gt_powersave(struct drm_device *dev)
valleyview_disable_rps(dev);
else
gen6_disable_rps(dev);
+   dev_priv-rps.enabled = false;
mutex_unlock(dev_priv-rps.hw_lock);
}
 }
@@ -4676,6 +4681,7 @@ static void intel_gen6_powersave_work(struct work_struct 
*work)
gen6_enable_rps(dev);
gen6_update_ring_freq(dev);
}
+   dev_priv-rps.enabled = true;
mutex_unlock(dev_priv-rps.hw_lock);
 }
 
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Avoid tweaking RPS before it is enabled

2013-10-10 Thread Jesse Barnes
On Thu, 10 Oct 2013 21:58:50 +0100
Chris Wilson ch...@chris-wilson.co.uk wrote:

 As we delay the initial RPS enabling (upon boot and after resume), there
 is a chance that we may start to render and trigger RPS boosts before we
 set up the punit. Any changes we make could result in inconsistent
 hardware state, with a danger of causing undefined behaviour. However,
 as the boosting is a optional tweak to RPS, we can simply ignore it
 whilst RPS is not yet enabled.
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Daniel Vetter daniel.vet...@ffwll.ch
 Cc: Jesse Barnes jbar...@virtuousgeek.org
 ---
  drivers/gpu/drm/i915/i915_drv.h |1 +
  drivers/gpu/drm/i915/intel_pm.c |   26 --
  2 files changed, 17 insertions(+), 10 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 640bff2..e0152e7 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -874,6 +874,7 @@ struct intel_gen6_power_mgmt {
   int last_adj;
   enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
  
 + bool enabled;
   struct delayed_work delayed_resume_work;
  
   /*
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 6ffeb04..8070a07 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -3435,22 +3435,26 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
  void gen6_rps_idle(struct drm_i915_private *dev_priv)
  {
   mutex_lock(dev_priv-rps.hw_lock);
 - if (dev_priv-info-is_valleyview)
 - valleyview_set_rps(dev_priv-dev, dev_priv-rps.min_delay);
 - else
 - gen6_set_rps(dev_priv-dev, dev_priv-rps.min_delay);
 - dev_priv-rps.last_adj = 0;
 + if (dev_priv-rps.enabled) {
 + if (dev_priv-info-is_valleyview)
 + valleyview_set_rps(dev_priv-dev, 
 dev_priv-rps.min_delay);
 + else
 + gen6_set_rps(dev_priv-dev, dev_priv-rps.min_delay);
 + dev_priv-rps.last_adj = 0;
 + }
   mutex_unlock(dev_priv-rps.hw_lock);
  }
  
  void gen6_rps_boost(struct drm_i915_private *dev_priv)
  {
   mutex_lock(dev_priv-rps.hw_lock);
 - if (dev_priv-info-is_valleyview)
 - valleyview_set_rps(dev_priv-dev, dev_priv-rps.max_delay);
 - else
 - gen6_set_rps(dev_priv-dev, dev_priv-rps.max_delay);
 - dev_priv-rps.last_adj = 0;
 + if (dev_priv-rps.enabled) {
 + if (dev_priv-info-is_valleyview)
 + valleyview_set_rps(dev_priv-dev, 
 dev_priv-rps.max_delay);
 + else
 + gen6_set_rps(dev_priv-dev, dev_priv-rps.max_delay);
 + dev_priv-rps.last_adj = 0;
 + }
   mutex_unlock(dev_priv-rps.hw_lock);
  }
  
 @@ -4657,6 +4661,7 @@ void intel_disable_gt_powersave(struct drm_device *dev)
   valleyview_disable_rps(dev);
   else
   gen6_disable_rps(dev);
 + dev_priv-rps.enabled = false;
   mutex_unlock(dev_priv-rps.hw_lock);
   }
  }
 @@ -4676,6 +4681,7 @@ static void intel_gen6_powersave_work(struct 
 work_struct *work)
   gen6_enable_rps(dev);
   gen6_update_ring_freq(dev);
   }
 + dev_priv-rps.enabled = true;
   mutex_unlock(dev_priv-rps.hw_lock);
  }
  

Yeah looks good.  Probably better than doing a sync on the delayed work
too, since that'll take over 1s.

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Fix pipe off timeout handling for pre-gen4

2013-10-10 Thread Daniel Vetter
On Thu, Oct 10, 2013 at 09:42:44PM +0100, Chris Wilson wrote:
 On Thu, Oct 10, 2013 at 08:32:23PM +0300, ville.syrj...@linux.intel.com wrote:
  From: Ville Syrjälä ville.syrj...@linux.intel.com
  
  The current pre-gen4 pipe off code might break out of the loop
  due to the timeout, but then the fail to print the warning.
  
  Fix the issue by making sure we pair up the correct time comparison
  functions. It would be enough to change just the final check to use
  time_after_eq(), but also changing the loop condition to use
  time_before() instead of time_after() makes the whole thing look a
  bit saner since then we always compare jiffies and timeout in the same
  order, and the words before and after now feel more natural when
  talking about the timeout.
 
 Don't you also want to recheck last_line just in case it managed to
 finish in the nick of time?

Indeed, this is the same bug as with all those waits - we need to recheck
the condition after the wait in case the scheduler was a bully and didn't
give us any cpu time before the timeout elapsed.

I'll drop the patch again and hide in shame ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Avoid tweaking RPS before it is enabled

2013-10-10 Thread Daniel Vetter
On Thu, Oct 10, 2013 at 02:06:02PM -0700, Jesse Barnes wrote:
 On Thu, 10 Oct 2013 21:58:50 +0100
 Chris Wilson ch...@chris-wilson.co.uk wrote:
 
  As we delay the initial RPS enabling (upon boot and after resume), there
  is a chance that we may start to render and trigger RPS boosts before we
  set up the punit. Any changes we make could result in inconsistent
  hardware state, with a danger of causing undefined behaviour. However,
  as the boosting is a optional tweak to RPS, we can simply ignore it
  whilst RPS is not yet enabled.
  
  Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
  Cc: Daniel Vetter daniel.vet...@ffwll.ch
  Cc: Jesse Barnes jbar...@virtuousgeek.org
  ---
   drivers/gpu/drm/i915/i915_drv.h |1 +
   drivers/gpu/drm/i915/intel_pm.c |   26 --
   2 files changed, 17 insertions(+), 10 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/i915_drv.h 
  b/drivers/gpu/drm/i915/i915_drv.h
  index 640bff2..e0152e7 100644
  --- a/drivers/gpu/drm/i915/i915_drv.h
  +++ b/drivers/gpu/drm/i915/i915_drv.h
  @@ -874,6 +874,7 @@ struct intel_gen6_power_mgmt {
  int last_adj;
  enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
   
  +   bool enabled;
  struct delayed_work delayed_resume_work;
   
  /*
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index 6ffeb04..8070a07 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -3435,22 +3435,26 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
   void gen6_rps_idle(struct drm_i915_private *dev_priv)
   {
  mutex_lock(dev_priv-rps.hw_lock);
  -   if (dev_priv-info-is_valleyview)
  -   valleyview_set_rps(dev_priv-dev, dev_priv-rps.min_delay);
  -   else
  -   gen6_set_rps(dev_priv-dev, dev_priv-rps.min_delay);
  -   dev_priv-rps.last_adj = 0;
  +   if (dev_priv-rps.enabled) {
  +   if (dev_priv-info-is_valleyview)
  +   valleyview_set_rps(dev_priv-dev, 
  dev_priv-rps.min_delay);
  +   else
  +   gen6_set_rps(dev_priv-dev, dev_priv-rps.min_delay);
  +   dev_priv-rps.last_adj = 0;
  +   }
  mutex_unlock(dev_priv-rps.hw_lock);
   }
   
   void gen6_rps_boost(struct drm_i915_private *dev_priv)
   {
  mutex_lock(dev_priv-rps.hw_lock);
  -   if (dev_priv-info-is_valleyview)
  -   valleyview_set_rps(dev_priv-dev, dev_priv-rps.max_delay);
  -   else
  -   gen6_set_rps(dev_priv-dev, dev_priv-rps.max_delay);
  -   dev_priv-rps.last_adj = 0;
  +   if (dev_priv-rps.enabled) {
  +   if (dev_priv-info-is_valleyview)
  +   valleyview_set_rps(dev_priv-dev, 
  dev_priv-rps.max_delay);
  +   else
  +   gen6_set_rps(dev_priv-dev, dev_priv-rps.max_delay);
  +   dev_priv-rps.last_adj = 0;
  +   }
  mutex_unlock(dev_priv-rps.hw_lock);
   }
   
  @@ -4657,6 +4661,7 @@ void intel_disable_gt_powersave(struct drm_device 
  *dev)
  valleyview_disable_rps(dev);
  else
  gen6_disable_rps(dev);
  +   dev_priv-rps.enabled = false;
  mutex_unlock(dev_priv-rps.hw_lock);
  }
   }
  @@ -4676,6 +4681,7 @@ static void intel_gen6_powersave_work(struct 
  work_struct *work)
  gen6_enable_rps(dev);
  gen6_update_ring_freq(dev);
  }
  +   dev_priv-rps.enabled = true;
  mutex_unlock(dev_priv-rps.hw_lock);
   }
   
 
 Yeah looks good.  Probably better than doing a sync on the delayed work
 too, since that'll take over 1s.
 
 Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2 01/16] drm/i915: Add intel_pipe_wm and prepare for watermark pre-compute

2013-10-10 Thread Paulo Zanoni
2013/10/9  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 Introduce a new struct intel_pipe_wm which contains all the
 watermarks for a single pipe. Use it to unify the LP0 and LP1+
 watermark computations so that we can just iterate through the
 watermark levels neatly and call ilk_compute_wm_level() for each.

 Also add another tool ilk_wm_merge() that merges the LP1+ watermarks
 from all pipes. For that, embed one intel_pipe_wm inside intel_crtc that
 contains the currently valid watermarks for each pipe.

 This is mainly preparatory work for pre-computing the watermarks for
 each pipe and merging them at a later time. For now the merging still
 happens immediately.

 v2: Add some comments about level 0 DDB split and intel_wm_config
 Add WARN_ON for level 0 being disabled
 s/lp_wm/merged

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/intel_drv.h |  12 +++
  drivers/gpu/drm/i915/intel_pm.c  | 192 
 +++
  2 files changed, 128 insertions(+), 76 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_drv.h 
 b/drivers/gpu/drm/i915/intel_drv.h
 index 71cfabd..3325b0b 100644
 --- a/drivers/gpu/drm/i915/intel_drv.h
 +++ b/drivers/gpu/drm/i915/intel_drv.h
 @@ -309,6 +309,12 @@ struct intel_crtc_config {
 bool double_wide;
  };

 +struct intel_pipe_wm {
 +   struct intel_wm_level wm[5];
 +   uint32_t linetime;
 +   bool fbc_wm_enabled;
 +};
 +
  struct intel_crtc {
 struct drm_crtc base;
 enum pipe pipe;
 @@ -349,6 +355,12 @@ struct intel_crtc {
 /* Access to these should be protected by dev_priv-irq_lock. */
 bool cpu_fifo_underrun_disabled;
 bool pch_fifo_underrun_disabled;
 +
 +   /* per-pipe watermark state */
 +   struct {
 +   /* watermarks currently being used  */
 +   struct intel_pipe_wm active;
 +   } wm;
  };

  struct intel_plane_wm_parameters {
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 5e743ec..30b380c 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2458,53 +2458,6 @@ static void ilk_compute_wm_level(struct 
 drm_i915_private *dev_priv,
 result-enable = true;
  }

 -static bool hsw_compute_lp_wm(struct drm_i915_private *dev_priv,
 - int level, const struct hsw_wm_maximums *max,
 - const struct hsw_pipe_wm_parameters *params,
 - struct intel_wm_level *result)
 -{
 -   enum pipe pipe;
 -   struct intel_wm_level res[3];
 -
 -   for (pipe = PIPE_A; pipe = PIPE_C; pipe++)
 -   ilk_compute_wm_level(dev_priv, level, params[pipe], 
 res[pipe]);
 -
 -   result-pri_val = max3(res[0].pri_val, res[1].pri_val, 
 res[2].pri_val);
 -   result-spr_val = max3(res[0].spr_val, res[1].spr_val, 
 res[2].spr_val);
 -   result-cur_val = max3(res[0].cur_val, res[1].cur_val, 
 res[2].cur_val);
 -   result-fbc_val = max3(res[0].fbc_val, res[1].fbc_val, 
 res[2].fbc_val);
 -   result-enable = true;
 -
 -   return ilk_check_wm(level, max, result);
 -}
 -
 -
 -static uint32_t hsw_compute_wm_pipe(struct drm_device *dev,
 -   const struct hsw_pipe_wm_parameters 
 *params)
 -{
 -   struct drm_i915_private *dev_priv = dev-dev_private;
 -   struct intel_wm_config config = {
 -   .num_pipes_active = 1,
 -   .sprites_enabled = params-spr.enabled,
 -   .sprites_scaled = params-spr.scaled,
 -   };
 -   struct hsw_wm_maximums max;
 -   struct intel_wm_level res;
 -
 -   if (!params-active)
 -   return 0;
 -
 -   ilk_wm_max(dev, 0, config, INTEL_DDB_PART_1_2, max);
 -
 -   ilk_compute_wm_level(dev_priv, 0, params, res);
 -
 -   ilk_check_wm(0, max, res);
 -
 -   return (res.pri_val  WM0_PIPE_PLANE_SHIFT) |
 -  (res.spr_val  WM0_PIPE_SPRITE_SHIFT) |
 -  res.cur_val;
 -}
 -
  static uint32_t
  hsw_compute_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
  {
 @@ -2687,44 +2640,123 @@ static void hsw_compute_wm_parameters(struct 
 drm_device *dev,
 *lp_max_5_6 = *lp_max_1_2;
  }

 +/* Compute new watermarks for the pipe */
 +static bool intel_compute_pipe_wm(struct drm_crtc *crtc,
 + const struct hsw_pipe_wm_parameters *params,
 + struct intel_pipe_wm *pipe_wm)
 +{
 +   struct drm_device *dev = crtc-dev;
 +   struct drm_i915_private *dev_priv = dev-dev_private;
 +   int level, max_level = ilk_wm_max_level(dev);
 +   /* LP0 watermark maximums depend on this pipe alone */
 +   struct intel_wm_config config = {
 +   .num_pipes_active = 1,
 +   .sprites_enabled = params-spr.enabled,
 +   .sprites_scaled = params-spr.scaled,
 

Re: [Intel-gfx] [PATCH 02/16] drm/i915: Don't re-compute pipe watermarks except for the affected pipe

2013-10-10 Thread Paulo Zanoni
2013/10/9  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 No point in re-computing the watermarks for all pipes, when only one
 pipe has changed. The watermarks stored under intel_crtc.wm.active are
 still valid for the other pipes. We just need to redo the merging.

 We can also skip the merge/update procedure completely if the new
 watermarks for the affected pipe come out unchanged.

Nice one!

Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com


 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/intel_pm.c | 67 
 +
  1 file changed, 28 insertions(+), 39 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 30b380c..7eea69d 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2584,29 +2584,19 @@ static void intel_setup_wm_latency(struct drm_device 
 *dev)
 intel_print_wm_latency(dev, Cursor, dev_priv-wm.cur_latency);
  }

 -static void hsw_compute_wm_parameters(struct drm_device *dev,
 - struct hsw_pipe_wm_parameters *params,
 +static void hsw_compute_wm_parameters(struct drm_crtc *crtc,
 + struct hsw_pipe_wm_parameters *p,
   struct hsw_wm_maximums *lp_max_1_2,
   struct hsw_wm_maximums *lp_max_5_6)
  {
 -   struct drm_crtc *crtc;
 -   struct drm_plane *plane;
 -   enum pipe pipe;
 +   struct drm_device *dev = crtc-dev;
 +   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 +   enum pipe pipe = intel_crtc-pipe;
 struct intel_wm_config config = {};
 +   struct drm_plane *plane;

 -   list_for_each_entry(crtc, dev-mode_config.crtc_list, head) {
 -   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 -   struct hsw_pipe_wm_parameters *p;
 -
 -   pipe = intel_crtc-pipe;
 -   p = params[pipe];
 -
 -   p-active = intel_crtc_active(crtc);
 -   if (!p-active)
 -   continue;
 -
 -   config.num_pipes_active++;
 -
 +   p-active = intel_crtc_active(crtc);
 +   if (p-active) {
 p-pipe_htotal = intel_crtc-config.adjusted_mode.htotal;
 p-pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
 p-pri.bytes_per_pixel = crtc-fb-bits_per_pixel / 8;
 @@ -2618,17 +2608,17 @@ static void hsw_compute_wm_parameters(struct 
 drm_device *dev,
 p-cur.enabled = true;
 }

 +   list_for_each_entry(crtc, dev-mode_config.crtc_list, head)
 +   config.num_pipes_active += intel_crtc_active(crtc);
 +
 list_for_each_entry(plane, dev-mode_config.plane_list, head) {
 struct intel_plane *intel_plane = to_intel_plane(plane);
 -   struct hsw_pipe_wm_parameters *p;
 -
 -   pipe = intel_plane-pipe;
 -   p = params[pipe];

 -   p-spr = intel_plane-wm;
 +   if (intel_plane-pipe == pipe)
 +   p-spr = intel_plane-wm;

 -   config.sprites_enabled |= p-spr.enabled;
 -   config.sprites_scaled |= p-spr.scaled;
 +   config.sprites_enabled |= intel_plane-wm.enabled;
 +   config.sprites_scaled |= intel_plane-wm.scaled;
 }

 ilk_wm_max(dev, 1, config, INTEL_DDB_PART_1_2, lp_max_1_2);
 @@ -2656,8 +2646,6 @@ static bool intel_compute_pipe_wm(struct drm_crtc *crtc,
 };
 struct hsw_wm_maximums max;

 -   memset(pipe_wm, 0, sizeof(*pipe_wm));
 -
 /* LP0 watermarks always use 1/2 DDB partitioning */
 ilk_wm_max(dev, 0, config, INTEL_DDB_PART_1_2, max);

 @@ -2728,7 +2716,6 @@ static void ilk_wm_merge(struct drm_device *dev,
  }

  static void hsw_compute_wm_results(struct drm_device *dev,
 -  const struct hsw_pipe_wm_parameters 
 *params,
const struct hsw_wm_maximums *lp_maximums,
struct hsw_wm_values *results)
  {
 @@ -2736,11 +2723,6 @@ static void hsw_compute_wm_results(struct drm_device 
 *dev,
 int level, wm_lp;
 struct intel_pipe_wm merged = {};

 -   list_for_each_entry(intel_crtc, dev-mode_config.crtc_list, 
 base.head)
 -   intel_compute_pipe_wm(intel_crtc-base,
 - params[intel_crtc-pipe],
 - intel_crtc-wm.active);
 -
 ilk_wm_merge(dev, lp_maximums, merged);

 memset(results, 0, sizeof(*results));
 @@ -2907,20 +2889,27 @@ static void hsw_write_wm_values(struct 
 drm_i915_private *dev_priv,

  static void haswell_update_wm(struct drm_crtc *crtc)
  {
 +   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 struct drm_device *dev = crtc-dev;
   

Re: [Intel-gfx] [PATCH 03/16] drm/i915: Move LP1+ watermark merging out from hsw_compute_wm_results()

2013-10-10 Thread Paulo Zanoni
2013/10/9  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 I want to convert hsw_find_best_result() to use intel_pipe_wm, so we
 need to move the merging to happen outside hsw_compute_wm_results().

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/intel_pm.c | 19 ++-
  1 file changed, 10 insertions(+), 9 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 7eea69d..c2d439f 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2716,26 +2716,23 @@ static void ilk_wm_merge(struct drm_device *dev,
  }

  static void hsw_compute_wm_results(struct drm_device *dev,
 -  const struct hsw_wm_maximums *lp_maximums,
 +  const struct intel_pipe_wm *merged,
struct hsw_wm_values *results)
  {
 struct intel_crtc *intel_crtc;
 int level, wm_lp;
 -   struct intel_pipe_wm merged = {};
 -
 -   ilk_wm_merge(dev, lp_maximums, merged);

 memset(results, 0, sizeof(*results));

 -   results-enable_fbc_wm = merged.fbc_wm_enabled;
 +   results-enable_fbc_wm = merged-fbc_wm_enabled;

 /* LP1+ register values */
 for (wm_lp = 1; wm_lp = 3; wm_lp++) {
 const struct intel_wm_level *r;

 -   level = wm_lp + (wm_lp = 2  merged.wm[4].enable);
 +   level = wm_lp + (wm_lp = 2  merged-wm[4].enable);

 -   r = merged.wm[level];
 +   r = merged-wm[level];
 if (!r-enable)
 break;

 @@ -2897,6 +2894,7 @@ static void haswell_update_wm(struct drm_crtc *crtc)
 struct hsw_wm_values results_1_2, results_5_6, *best_results;
 enum intel_ddb_partitioning partitioning;
 struct intel_pipe_wm pipe_wm = {};
 +   struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {};

 hsw_compute_wm_parameters(crtc, params, lp_max_1_2, lp_max_5_6);

 @@ -2907,9 +2905,12 @@ static void haswell_update_wm(struct drm_crtc *crtc)

 intel_crtc-wm.active = pipe_wm;

 -   hsw_compute_wm_results(dev, lp_max_1_2, results_1_2);
 +   ilk_wm_merge(dev, lp_max_1_2, lp_wm_1_2);
 +   ilk_wm_merge(dev, lp_max_5_6, lp_wm_5_6);

Shouldn't you move the 5_6 ilk_wm_merge to inside the if statement,
because it's only needed there?

With or without that: Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com

 +
 +   hsw_compute_wm_results(dev, lp_wm_1_2, results_1_2);
 if (lp_max_1_2.pri != lp_max_5_6.pri) {
 -   hsw_compute_wm_results(dev, lp_max_5_6, results_5_6);
 +   hsw_compute_wm_results(dev, lp_wm_5_6, results_5_6);
 best_results = hsw_find_best_result(results_1_2, 
 results_5_6);
 } else {
 best_results = results_1_2;
 --
 1.8.1.5

 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 04/16] drm/i915: Use intel_pipe_wm in hsw_find_best_results

2013-10-10 Thread Paulo Zanoni
2013/10/9  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 Let's try to keep using the intermediate intel_pipe_wm representation
 for as long as possible. It avoids subtle knowledge about the
 internals of the hardware registers when trying to choose the
 best watermark configuration.

 While at it replace the memset() w/ zero initialization.

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com

Looks correct. These things are hard to review... So many structs!

Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com

 ---
  drivers/gpu/drm/i915/intel_pm.c | 42 
 -
  1 file changed, 21 insertions(+), 21 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index c2d439f..b09715f 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2722,8 +2722,6 @@ static void hsw_compute_wm_results(struct drm_device 
 *dev,
 struct intel_crtc *intel_crtc;
 int level, wm_lp;

 -   memset(results, 0, sizeof(*results));
 -
 results-enable_fbc_wm = merged-fbc_wm_enabled;

 /* LP1+ register values */
 @@ -2763,24 +2761,26 @@ static void hsw_compute_wm_results(struct drm_device 
 *dev,

  /* Find the result with the highest level enabled. Check for enable_fbc_wm in
   * case both are at the same level. Prefer r1 in case they're the same. */
 -static struct hsw_wm_values *hsw_find_best_result(struct hsw_wm_values *r1,
 - struct hsw_wm_values *r2)
 +static struct intel_pipe_wm *hsw_find_best_result(struct drm_device *dev,
 + struct intel_pipe_wm *r1,
 + struct intel_pipe_wm *r2)
  {
 -   int i, val_r1 = 0, val_r2 = 0;
 +   int level, max_level = ilk_wm_max_level(dev);
 +   int level1 = 0, level2 = 0;

 -   for (i = 0; i  3; i++) {
 -   if (r1-wm_lp[i]  WM3_LP_EN)
 -   val_r1 = r1-wm_lp[i]  WM1_LP_LATENCY_MASK;
 -   if (r2-wm_lp[i]  WM3_LP_EN)
 -   val_r2 = r2-wm_lp[i]  WM1_LP_LATENCY_MASK;
 +   for (level = 1; level = max_level; level++) {
 +   if (r1-wm[level].enable)
 +   level1 = level;
 +   if (r2-wm[level].enable)
 +   level2 = level;
 }

 -   if (val_r1 == val_r2) {
 -   if (r2-enable_fbc_wm  !r1-enable_fbc_wm)
 +   if (level1 == level2) {
 +   if (r2-fbc_wm_enabled  !r1-fbc_wm_enabled)
 return r2;
 else
 return r1;
 -   } else if (val_r1  val_r2) {
 +   } else if (level1  level2) {
 return r1;
 } else {
 return r2;
 @@ -2891,10 +2891,10 @@ static void haswell_update_wm(struct drm_crtc *crtc)
 struct drm_i915_private *dev_priv = dev-dev_private;
 struct hsw_wm_maximums lp_max_1_2, lp_max_5_6;
 struct hsw_pipe_wm_parameters params = {};
 -   struct hsw_wm_values results_1_2, results_5_6, *best_results;
 +   struct hsw_wm_values results = {};
 enum intel_ddb_partitioning partitioning;
 struct intel_pipe_wm pipe_wm = {};
 -   struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {};
 +   struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;

 hsw_compute_wm_parameters(crtc, params, lp_max_1_2, lp_max_5_6);

 @@ -2908,18 +2908,18 @@ static void haswell_update_wm(struct drm_crtc *crtc)
 ilk_wm_merge(dev, lp_max_1_2, lp_wm_1_2);
 ilk_wm_merge(dev, lp_max_5_6, lp_wm_5_6);

 -   hsw_compute_wm_results(dev, lp_wm_1_2, results_1_2);
 if (lp_max_1_2.pri != lp_max_5_6.pri) {
 -   hsw_compute_wm_results(dev, lp_wm_5_6, results_5_6);
 -   best_results = hsw_find_best_result(results_1_2, 
 results_5_6);
 +   best_lp_wm = hsw_find_best_result(dev, lp_wm_1_2, 
 lp_wm_5_6);
 } else {
 -   best_results = results_1_2;
 +   best_lp_wm = lp_wm_1_2;
 }

 -   partitioning = (best_results == results_1_2) ?
 +   hsw_compute_wm_results(dev, best_lp_wm, results);
 +
 +   partitioning = (best_lp_wm == lp_wm_1_2) ?
INTEL_DDB_PART_1_2 : INTEL_DDB_PART_5_6;

 -   hsw_write_wm_values(dev_priv, best_results, partitioning);
 +   hsw_write_wm_values(dev_priv, results, partitioning);
  }

  static void haswell_update_sprite_wm(struct drm_plane *plane,
 --
 1.8.1.5

 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 05/16] drm/i915: Move some computations out from hsw_compute_wm_parameters()

2013-10-10 Thread Paulo Zanoni
2013/10/9  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 Move the watermark max computations into haswell_update_wm(). This
 allows keeping the 1/2 vs. 5/6 split code in one place, and avoid having
 to pass around so many things. We also save a bit of stack space by only
 requiring one copy of struct hsw_wm_maximums.

 Also move the intel_wm_config out from hsw_compute_wm_parameters() and
 pass it it. We'll have some need for it in haswell_update_wm() later.

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/intel_pm.c | 33 ++---
  1 file changed, 14 insertions(+), 19 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index b09715f..0fe6c36 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2586,13 +2586,11 @@ static void intel_setup_wm_latency(struct drm_device 
 *dev)

  static void hsw_compute_wm_parameters(struct drm_crtc *crtc,
   struct hsw_pipe_wm_parameters *p,
 - struct hsw_wm_maximums *lp_max_1_2,
 - struct hsw_wm_maximums *lp_max_5_6)
 + struct intel_wm_config *config)
  {
 struct drm_device *dev = crtc-dev;
 struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 enum pipe pipe = intel_crtc-pipe;
 -   struct intel_wm_config config = {};
 struct drm_plane *plane;

 p-active = intel_crtc_active(crtc);
 @@ -2609,7 +2607,7 @@ static void hsw_compute_wm_parameters(struct drm_crtc 
 *crtc,
 }

 list_for_each_entry(crtc, dev-mode_config.crtc_list, head)
 -   config.num_pipes_active += intel_crtc_active(crtc);
 +   config-num_pipes_active += intel_crtc_active(crtc);

 list_for_each_entry(plane, dev-mode_config.plane_list, head) {
 struct intel_plane *intel_plane = to_intel_plane(plane);
 @@ -2617,17 +2615,9 @@ static void hsw_compute_wm_parameters(struct drm_crtc 
 *crtc,
 if (intel_plane-pipe == pipe)
 p-spr = intel_plane-wm;

 -   config.sprites_enabled |= intel_plane-wm.enabled;
 -   config.sprites_scaled |= intel_plane-wm.scaled;
 +   config-sprites_enabled |= intel_plane-wm.enabled;
 +   config-sprites_scaled |= intel_plane-wm.scaled;
 }
 -
 -   ilk_wm_max(dev, 1, config, INTEL_DDB_PART_1_2, lp_max_1_2);
 -
 -   /* 5/6 split only in single pipe config on IVB+ */
 -   if (INTEL_INFO(dev)-gen = 7  config.num_pipes_active = 1)
 -   ilk_wm_max(dev, 1, config, INTEL_DDB_PART_5_6, lp_max_5_6);
 -   else
 -   *lp_max_5_6 = *lp_max_1_2;
  }

  /* Compute new watermarks for the pipe */
 @@ -2889,14 +2879,15 @@ static void haswell_update_wm(struct drm_crtc *crtc)
 struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 struct drm_device *dev = crtc-dev;
 struct drm_i915_private *dev_priv = dev-dev_private;
 -   struct hsw_wm_maximums lp_max_1_2, lp_max_5_6;
 +   struct hsw_wm_maximums max;
 struct hsw_pipe_wm_parameters params = {};
 struct hsw_wm_values results = {};
 enum intel_ddb_partitioning partitioning;
 struct intel_pipe_wm pipe_wm = {};
 struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;
 +   struct intel_wm_config config = {};

 -   hsw_compute_wm_parameters(crtc, params, lp_max_1_2, lp_max_5_6);
 +   hsw_compute_wm_parameters(crtc, params, config);

 intel_compute_pipe_wm(crtc, params, pipe_wm);

 @@ -2905,10 +2896,14 @@ static void haswell_update_wm(struct drm_crtc *crtc)

 intel_crtc-wm.active = pipe_wm;

 -   ilk_wm_merge(dev, lp_max_1_2, lp_wm_1_2);
 -   ilk_wm_merge(dev, lp_max_5_6, lp_wm_5_6);
 +   ilk_wm_max(dev, 1, config, INTEL_DDB_PART_1_2, max);
 +   ilk_wm_merge(dev, max, lp_wm_1_2);
 +
 +   /* 5/6 split only in single pipe config on IVB+ */
 +   if (INTEL_INFO(dev)-gen = 7  config.num_pipes_active = 1) {

No need to calculate 5_6 on zero pipes, I guess.


 +   ilk_wm_max(dev, 1, config, INTEL_DDB_PART_5_6, max);
 +   ilk_wm_merge(dev, max, lp_wm_5_6);

Oh, so now you've moved ilk_wm_merge to the if statement, as I
requested on the review to a previous patch :)


 -   if (lp_max_1_2.pri != lp_max_5_6.pri) {

By removing this check, you're now also calculating 5_6 watermarks for
the case where we just have 1 pipe but the sprites are disabled.


 best_lp_wm = hsw_find_best_result(dev, lp_wm_1_2, 
 lp_wm_5_6);
 } else {
 best_lp_wm = lp_wm_1_2;
 --
 1.8.1.5

 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 

Re: [Intel-gfx] [PATCH 06/16] drm/i915: Don't compute 5/6 DDB split w/ zero active pipes

2013-10-10 Thread Paulo Zanoni
2013/10/9  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 When there are zero active pipes, all the watermarks should be zero
 also. No point in wasting time w/ computing the 5/6 split watermark
 config.

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/intel_pm.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 0fe6c36..c17518d 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2900,7 +2900,7 @@ static void haswell_update_wm(struct drm_crtc *crtc)
 ilk_wm_merge(dev, max, lp_wm_1_2);

 /* 5/6 split only in single pipe config on IVB+ */
 -   if (INTEL_INFO(dev)-gen = 7  config.num_pipes_active = 1) {
 +   if (INTEL_INFO(dev)-gen = 7  config.num_pipes_active == 1) {

I was going to say but you've just introduced this in the last
patch, then I re-check and saw that it was already there even before
the previous patch, you've just moved the code around on the last
patch.

Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com

 ilk_wm_max(dev, 1, config, INTEL_DDB_PART_5_6, max);
 ilk_wm_merge(dev, max, lp_wm_5_6);

 --
 1.8.1.5

 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 07/16] drm/i915: Refactor wm_lp to level calculation

2013-10-10 Thread Paulo Zanoni
2013/10/9  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 On HSW the LP1,LP2,LP3 levels are either 1,2,3 or 1,3,4. We make the
 conversion from LPn to to the level at one point current. Later we're
 going to do it in a few places, so move it to a separate function.

I guess this function will work on ILK/SNB/IVB even though they don't
follow this rule, right? If yes: Reviewed-by: Paulo Zanoni
paulo.r.zan...@intel.com


 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/intel_pm.c | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index c17518d..d307039 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2705,6 +2705,12 @@ static void ilk_wm_merge(struct drm_device *dev,
 }
  }

 +static int ilk_wm_lp_to_level(int wm_lp, const struct intel_pipe_wm *pipe_wm)
 +{
 +   /* LP1,LP2,LP3 levels are either 1,2,3 or 1,3,4 */
 +   return wm_lp + (wm_lp = 2  pipe_wm-wm[4].enable);
 +}
 +
  static void hsw_compute_wm_results(struct drm_device *dev,
const struct intel_pipe_wm *merged,
struct hsw_wm_values *results)
 @@ -2718,7 +2724,7 @@ static void hsw_compute_wm_results(struct drm_device 
 *dev,
 for (wm_lp = 1; wm_lp = 3; wm_lp++) {
 const struct intel_wm_level *r;

 -   level = wm_lp + (wm_lp = 2  merged-wm[4].enable);
 +   level = ilk_wm_lp_to_level(wm_lp, merged);

 r = merged-wm[level];
 if (!r-enable)
 --
 1.8.1.5

 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 08/16] drm/i915: Kill fbc_wm_enabled from intel_wm_config

2013-10-10 Thread Paulo Zanoni
2013/10/9  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 The fbc_wm_enabled member in intel_wm_config is useless for the time
 being. The original idea for it was that we'd pre-compute it and so
 that the WM merging process could know whether it needs to worry
 about FBC watermarks at all.

If it compiles, then I guess it's fine, right?


 But we don't have a convenient way to pre-check for the possibility
 of FBC being used. intel_update_fbc() should be split up for that.

I wonder if we should keep that register bit which says disable FBC
watermarks disabled all the time while FBC is disabled... Last time I
checked, we were not doing this.

Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com


 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/intel_pm.c | 1 -
  1 file changed, 1 deletion(-)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index d307039..e81221d 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2213,7 +2213,6 @@ struct intel_wm_config {
 unsigned int num_pipes_active;
 bool sprites_enabled;
 bool sprites_scaled;
 -   bool fbc_wm_enabled;
  };

  /*
 --
 1.8.1.5

 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v4 3/4] ACPI / video: Do not register backlight if win8 and native interface exists

2013-10-10 Thread Aaron Lu
On 10/10/2013 08:59 PM, Rafael J. Wysocki wrote:
 On Thursday, October 10, 2013 09:02:55 AM Aaron Lu wrote:
 On 10/10/2013 08:29 AM, Rafael J. Wysocki wrote:
 On Tuesday, October 08, 2013 02:40:00 PM Aaron Lu wrote:
 According to Matthew Garrett, Windows 8 leaves backlight control up
 to individual graphics drivers rather than making ACPI calls itself.
 There's plenty of evidence to suggest that the Intel driver for
 Windows [8] doesn't use the ACPI interface, including the fact that
 it's broken on a bunch of machines when the OS claims to support
 Windows 8.  The simplest thing to do appears to be to disable the
 ACPI backlight interface on these systems.

 So for Win8 systems, if there is native backlight control interface
 registered by GPU driver, ACPI video will not register its own. For
 users who prefer to keep ACPI video's backlight interface, the existing
 kernel cmdline option acpi_backlight=video can be used.

 Signed-off-by: Aaron Lu aaron...@intel.com
 Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com
 Tested-by: Yves-Alexis Perez cor...@debian.org
 Tested-by: Mika Westerberg mika.westerb...@linux.intel.com
 ---
  drivers/acpi/internal.h |  5 ++---
  drivers/acpi/video.c| 10 +-
  drivers/acpi/video_detect.c | 14 --
  3 files changed, 19 insertions(+), 10 deletions(-)

 diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
 index 20f4233..453ae8d 100644
 --- a/drivers/acpi/internal.h
 +++ b/drivers/acpi/internal.h
 @@ -169,9 +169,8 @@ int acpi_create_platform_device(struct acpi_device 
 *adev,
Video

 -- 
 */
  #if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
 -bool acpi_video_backlight_quirks(void);
 -#else
 -static inline bool acpi_video_backlight_quirks(void) { return false; }
 +bool acpi_osi_is_win8(void);
 +bool acpi_video_verify_backlight_support(void);
  #endif
  
  #endif /* _ACPI_INTERNAL_H_ */
 diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
 index 3bd1eaa..343db59 100644
 --- a/drivers/acpi/video.c
 +++ b/drivers/acpi/video.c
 @@ -1256,8 +1256,8 @@ acpi_video_switch_brightness(struct 
 acpi_video_device *device, int event)
unsigned long long level_current, level_next;
int result = -EINVAL;
  
 -  /* no warning message if acpi_backlight=vendor is used */
 -  if (!acpi_video_backlight_support())
 +  /* no warning message if acpi_backlight=vendor or a quirk is used */
 +  if (!acpi_video_verify_backlight_support())
return 0;
  
if (!device-brightness)
 @@ -1386,13 +1386,13 @@ acpi_video_bus_get_devices(struct acpi_video_bus 
 *video,
  static int acpi_video_bus_start_devices(struct acpi_video_bus *video)
  {
return acpi_video_bus_DOS(video, 0,
 -acpi_video_backlight_quirks() ? 1 : 0);
 +acpi_osi_is_win8() ? 1 : 0);
  }
  
  static int acpi_video_bus_stop_devices(struct acpi_video_bus *video)
  {
return acpi_video_bus_DOS(video, 0,
 -acpi_video_backlight_quirks() ? 0 : 1);
 +acpi_osi_is_win8() ? 0 : 1);
  }
  
  static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
 @@ -1558,7 +1558,7 @@ acpi_video_bus_match(acpi_handle handle, u32 level, 
 void *context,
  
  static void acpi_video_dev_register_backlight(struct acpi_video_device 
 *device)
  {
 -  if (acpi_video_backlight_support()) {
 +  if (acpi_video_verify_backlight_support()) {
struct backlight_properties props;
struct pci_dev *pdev;
acpi_handle acpi_parent;
 diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
 index 940edbf..23d7d26 100644
 --- a/drivers/acpi/video_detect.c
 +++ b/drivers/acpi/video_detect.c
 @@ -37,6 +37,7 @@
  #include linux/acpi.h
  #include linux/dmi.h
  #include linux/pci.h
 +#include linux/backlight.h
  
  #include internal.h
  
 @@ -233,11 +234,11 @@ static void acpi_video_caps_check(void)
acpi_video_get_capabilities(NULL);
  }
  
 -bool acpi_video_backlight_quirks(void)
 +bool acpi_osi_is_win8(void)
  {
return acpi_gbl_osi_data = ACPI_OSI_WIN_8;
  }
 -EXPORT_SYMBOL(acpi_video_backlight_quirks);
 +EXPORT_SYMBOL(acpi_osi_is_win8);
  
  /* Promote the vendor interface instead of the generic video module.
   * This function allow DMI blacklists to be implemented by externals
 @@ -283,6 +284,15 @@ int acpi_video_backlight_support(void)
  }
  EXPORT_SYMBOL(acpi_video_backlight_support);
  
 +bool acpi_video_verify_backlight_support(void)
 +{
 +  if (!(acpi_video_support  ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO) 
 +  acpi_osi_is_win8()  backlight_device_registered(BACKLIGHT_RAW))
 +  return false;

 If I'm not mistaken, this will introduce a regression for the people who 
 have
 problems with the native i915 backlight on Win8-compatible systems.  I'd 
 prefer
 to avoid that at this point.


 OK, I 

[Intel-gfx] [PATCH intel-gpu-tools] configure: Don't bail if libdrm_nouveau isn't available.

2013-10-10 Thread Matt Turner
We were seriously *requiring* libdrm_nouveau unless explicitly disabled?
---
 configure.ac | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index f65942f..43740f9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -92,8 +92,11 @@ AM_CONDITIONAL(BUILD_ASSEMBLER, [test x$enable_assembler = 
xyes])
 # -
 # for dma-buf tests
 AC_ARG_ENABLE(nouveau, AS_HELP_STRING([--disable-nouveau],
- [Enable use of nouveau API for prime tests (default: enabled)]),
- [NOUVEAU=$enableval], [NOUVEAU=yes])
+ [Enable use of nouveau API for prime tests (default: auto)]),
+ [NOUVEAU=$enableval], [NOUVEAU=auto])
+if test x$NOUVEAU = xauto; then
+   PKG_CHECK_EXISTS([libdrm_nouveau = 2.4.33], [NOUVEAU=yes], 
[NOUVEAU=no])
+fi
 if test x$NOUVEAU = xyes; then
PKG_CHECK_MODULES(DRM_NOUVEAU, [libdrm_nouveau = 2.4.33])
AC_DEFINE(HAVE_NOUVEAU, 1, [Have nouveau support])
-- 
1.8.3.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH intel-gpu-tools] configure: Don't bail if libdrm_nouveau isn't available.

2013-10-10 Thread Ben Widawsky
On Thu, Oct 10, 2013 at 08:56:45PM -0700, Matt Turner wrote:
 We were seriously *requiring* libdrm_nouveau unless explicitly disabled?

Acked-by: Ben Widawsky b...@bwidawsk.net

 ---
  configure.ac | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)
 
 diff --git a/configure.ac b/configure.ac
 index f65942f..43740f9 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -92,8 +92,11 @@ AM_CONDITIONAL(BUILD_ASSEMBLER, [test x$enable_assembler 
 = xyes])
  # 
 -
  # for dma-buf tests
  AC_ARG_ENABLE(nouveau, AS_HELP_STRING([--disable-nouveau],
 -   [Enable use of nouveau API for prime tests (default: enabled)]),
 -   [NOUVEAU=$enableval], [NOUVEAU=yes])
 +   [Enable use of nouveau API for prime tests (default: auto)]),
 +   [NOUVEAU=$enableval], [NOUVEAU=auto])
 +if test x$NOUVEAU = xauto; then
 + PKG_CHECK_EXISTS([libdrm_nouveau = 2.4.33], [NOUVEAU=yes], 
 [NOUVEAU=no])
 +fi
  if test x$NOUVEAU = xyes; then
   PKG_CHECK_MODULES(DRM_NOUVEAU, [libdrm_nouveau = 2.4.33])
   AC_DEFINE(HAVE_NOUVEAU, 1, [Have nouveau support])
 -- 
 1.8.3.2
 

-- 
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Kconfig option to disable the legacy fbdev support

2013-10-10 Thread Lee, Chon Ming
On 10/09 09:18, Daniel Vetter wrote:
 Boots Just Fine (tm)!
 
 The only glitch seems to be that at least on Fedora the boot splash
 gets confused and doesn't display much at all.
 
 And since there's no ugly console flickering anymore in between, the
 flicker while switching between X servers (VT support is still enabled)
 is even more jarring.
 
 Also, I'm unsure whether we don't need to somehow kick out vgacon, now
 that nothing else gets in the way. But stuff seems to work, so I
 don't care. Also everything still works as well with VGA_CONSOLE=n
 
 Also the #ifdef mess needs a bit of a cleanup, follow-up patches will
 do just that.
 
 To keep the Kconfig tidy, extract all the i915 options into its own
 file.
 
 v2:
 - Rebase on top of the preliminary hw support option and the
   intel_drv.h cleanup.
 - Shut up warnings in i915_debugfs.c
 
 v3: Use the right CONFIG variable, spotted by Chon Ming.
 
 Cc: Lee, Chon Ming chon.ming@intel.com
 Cc: David Herrmann dh.herrm...@gmail.com
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---

Look good to me this series.

Reviewed-by: Chon Ming Lee chon.ming@intel.com

  drivers/gpu/drm/Kconfig  | 60 +---
  drivers/gpu/drm/i915/Kconfig | 67 
 
  drivers/gpu/drm/i915/Makefile|  3 +-
  drivers/gpu/drm/i915/i915_debugfs.c  |  9 ++---
  drivers/gpu/drm/i915/i915_dma.c  |  6 
  drivers/gpu/drm/i915/i915_drv.h  |  2 ++
  drivers/gpu/drm/i915/intel_display.c | 10 ++
  drivers/gpu/drm/i915/intel_drv.h | 36 +++
  8 files changed, 122 insertions(+), 71 deletions(-)
  create mode 100644 drivers/gpu/drm/i915/Kconfig
 
 diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
 index 3104b6d..b4e4fc0 100644
 --- a/drivers/gpu/drm/Kconfig
 +++ b/drivers/gpu/drm/Kconfig
 @@ -128,65 +128,7 @@ config DRM_I810
 selected, the module will be called i810.  AGP support is required
 for this driver to work.
  
 -config DRM_I915
 - tristate Intel 8xx/9xx/G3x/G4x/HD Graphics
 - depends on DRM
 - depends on AGP
 - depends on AGP_INTEL
 - # we need shmfs for the swappable backing store, and in particular
 - # the shmem_readpage() which depends upon tmpfs
 - select SHMEM
 - select TMPFS
 - select DRM_KMS_HELPER
 - select DRM_KMS_FB_HELPER
 - select FB_CFB_FILLRECT
 - select FB_CFB_COPYAREA
 - select FB_CFB_IMAGEBLIT
 - # i915 depends on ACPI_VIDEO when ACPI is enabled
 - # but for select to work, need to select ACPI_VIDEO's dependencies, ick
 - select BACKLIGHT_LCD_SUPPORT if ACPI
 - select BACKLIGHT_CLASS_DEVICE if ACPI
 - select VIDEO_OUTPUT_CONTROL if ACPI
 - select INPUT if ACPI
 - select THERMAL if ACPI
 - select ACPI_VIDEO if ACPI
 - select ACPI_BUTTON if ACPI
 - help
 -   Choose this option if you have a system that has Intel Graphics
 -   Media Accelerator or HD Graphics integrated graphics,
 -   including 830M, 845G, 852GM, 855GM, 865G, 915G, 945G, 965G,
 -   G35, G41, G43, G45 chipsets and Celeron, Pentium, Core i3,
 -   Core i5, Core i7 as well as Atom CPUs with integrated graphics.
 -   If M is selected, the module will be called i915.  AGP support
 -   is required for this driver to work. This driver is used by
 -   the Intel driver in X.org 6.8 and XFree86 4.4 and above. It
 -   replaces the older i830 module that supported a subset of the
 -   hardware in older X.org releases.
 -
 -   Note that the older i810/i815 chipsets require the use of the
 -   i810 driver instead, and the Atom z5xx series has an entirely
 -   different implementation.
 -
 -config DRM_I915_KMS
 - bool Enable modesetting on intel by default
 - depends on DRM_I915
 - help
 -   Choose this option if you want kernel modesetting enabled by default,
 -   and you have a new enough userspace to support this. Running old
 -   userspaces with this enabled will cause pain.  Note that this causes
 -   the driver to bind to PCI devices, which precludes loading things
 -   like intelfb.
 -
 -config DRM_I915_PRELIMINARY_HW_SUPPORT
 - bool Enable preliminary support for prerelease Intel hardware by 
 default
 - depends on DRM_I915
 - help
 -   Choose this option if you have prerelease Intel hardware and want the
 -   i915 driver to support it by default.  You can enable such support at
 -   runtime with the module option i915.preliminary_hw_support=1; this
 -   option changes the default for that module option.
 -
 -   If in doubt, say N.
 +source drivers/gpu/drm/i915/Kconfig
  
  config DRM_MGA
   tristate Matrox g200/g400
 diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
 new file mode 100644
 index 000..6199d0b
 --- /dev/null
 +++ b/drivers/gpu/drm/i915/Kconfig
 @@ -0,0 +1,67 @@
 +config DRM_I915
 + tristate Intel