Re: [Intel-gfx] drm/i915: Avoid accessing the stolen address when it is unavailable

2013-10-25 Thread Chris Wilson 
On Fri, Oct 25, 2013 at 12:27:42AM +, Liu, Chuansheng wrote:
> Hello Chris and Ben,
> 
> > -Original Message-
> > From: Ben Widawsky [mailto:b...@bwidawsk.net]
> > Sent: Friday, October 25, 2013 4:57 AM
> > To: Chris Wilson; Liu, Chuansheng; daniel.vet...@ffwll.ch; airl...@linux.ie;
> > intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org;
> > linux-ker...@vger.kernel.org; Li, Fei
> > Subject: Re: [Intel-gfx] drm/i915: Avoid accessing the stolen address when 
> > it is
> > unavailable
> > 
> > On Thu, Oct 24, 2013 at 01:17:06PM +0100, Chris Wilson wrote:
> > > On Fri, Oct 25, 2013 at 12:33:47AM +0800, Chuansheng Liu wrote:
> > > >
> > > > In our platform, we hit the the stolen region initialization failure 
> > > > case,
> > > > such as below log:
> > > > [drm:i915_stolen_to_physical] *ERROR* conflict detected with stolen
> > region: [0x7b00]
> > > >
> > > > And it causes the dev_priv->mm.stolen_base is NULL, in this case, we
> > should
> > > > avoid accessing it any more.
> > > >
> > > > Here is possible call trace:
> > > > intel_enable_gt_powersave -- >
> > > > valleyview_enable_rps -- >
> > > > valleyview_setup_pctx
> > >
> > > The two create_stolen routines are no-ops in that case so all that
> > > happens instead is that we read VLV_PCBR. However, really if
> > > i915_gem_object_create_stolen_for_preallocated() fails we should abort
> > > loading the driver as it means we have a hardware conflict and undefined
> > > behaviour.
> In case of dev_priv->mm.stolen_base == NULL, and the valleyview_setup_pctx() 
> is called
> at the first time, it will call 
> i915_gem_object_create_stolen_for_preallocated(), which should
> should return NULL always due to (!drm_mm_initialized(&dev_priv->mm.stolen)).
> 
> After that, every time specially when doing pm operation, the above scenario 
> will
> be called again and again.
> 
> Here this patch is to save some time for PM operation, we do not need to read
> VLV_PCBR and pcbr_offset calculation in case of stolen_base == NULL.
> 
> Is it making sense? Thanks.

I see. No, it is a pointless optimisation that leaks knowledge about
internals of another subsystem to paper over a kernel bug.
-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 for crashing intel server

2013-10-25 Thread Chris Wilson 
On Fri, Oct 25, 2013 at 05:46:53AM +0200, Bas Wijnen wrote:
> On Wed, Oct 23, 2013 at 09:28:28AM +0100, Chris Wilson wrote:
> > No worries, if you can run
> > 
> > addr2line -e /usr/lib/xorg/modules/drivers/intel_drv.so -i 0xfcd79 0xf8215
> > 
> > that should give me the information needed to pinpoint the crash.
> 
> $ addr2line -e /usr/lib/xorg/modules/drivers/intel_drv.so -i 0xfcd79
> 0xf8215
> /build/xserver-xorg-video-intel-WbV7Z9/xserver-xorg-video-intel-2.21.15/build/src/uxa/../../../src/uxa/intel.h:138
> /build/xserver-xorg-video-intel-WbV7Z9/xserver-xorg-video-intel-2.21.15/build/src/uxa/../../../src/uxa/i915_video.c:156
> /build/xserver-xorg-video-intel-WbV7Z9/xserver-xorg-video-intel-2.21.15/build/src/uxa/../../../src/uxa/intel_video.c:1584
> 
> Note that I'm running the unpatched Debian version again (so not with
> your or my patch), which is why it was crashing.

Ah. Ok, but we still don't know how we end up in this situation. If you
apply the patch to prevent the crash here, can you please report what
the contents of /sys/kernel/debug/dri/0/i915_gem_objects is at the time
the video goes black?
-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: HSW GT3 Slices: exec flag to warn kernel that userspace is using predication

2013-10-25 Thread Chris Wilson 
On Thu, Oct 24, 2013 at 06:24:18PM -0200, Rodrigo Vivi wrote:
> If Userspace isn't using MI_PREDICATE all slices must be enabled for
> backward compatibility.
> 
> If I915_EXEC_USE_PREDICATE isn't set and defaul is set to half, kernel will 
> force
> all slices on.
> 
> v2: fix the inverted logic for backwards compatibility
> USE_PREDICATE unset force gt_full when defaul is half
> instead of GT_FULL flag.
> 
> v3: Accepting Chris's suggestions: better variable names;
> better logic around state_default x legacy_userspace_busy;
> remove unecessary mutex;
> 
> v4: Accepting more suggestions from Chris:
> * Send all LRIs in only one block and don't ignore if it fails.
> * function name and cleaner code on forcing_full.
> 
> CC: Chris Wilson 
> CC: Eric Anholt 
> CC: Kenneth Graunke 
> Signed-off-by: Rodrigo Vivi 
> ---
>  drivers/gpu/drm/i915/i915_drv.h|  8 
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 63 
> ++
>  drivers/gpu/drm/i915/i915_reg.h| 11 ++
>  drivers/gpu/drm/i915/i915_sysfs.c  |  3 ++
>  drivers/gpu/drm/i915/intel_display.c   |  5 +++
>  drivers/gpu/drm/i915/intel_drv.h   |  1 +
>  drivers/gpu/drm/i915/intel_pm.c| 45 -
>  include/uapi/drm/i915_drm.h|  8 +++-
>  8 files changed, 141 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 685fb1d..67bbbce 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1219,6 +1219,12 @@ struct i915_package_c8 {
>   } regsave;
>  };
>  
> +struct i915_gt_slices {
> + int state_default;
> + int legacy_userspace_busy;
> + struct mutex lock; /* locks access to this scruct and slice registers */
> +};

Ok, I'm getting happier. This time I looked at what locks we were
holding for accessing state_default and legacy_userspace_busy. I'm
afraid we alternated between no locking and struct_mutex. This will add
some unfortunate complexity...

> +
>  typedef struct drm_i915_private {
>   struct drm_device *dev;
>   struct kmem_cache *slab;
> @@ -1418,6 +1424,8 @@ typedef struct drm_i915_private {
>  
>   struct i915_package_c8 pc8;
>  
> + struct i915_gt_slices gt_slices;
> +
>   /* Old dri1 support infrastructure, beware the dragons ya fools entering
>* here! */
>   struct i915_dri1_state dri1;
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 0ce0d47..7a362a2 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -922,6 +922,62 @@ i915_reset_gen7_sol_offsets(struct drm_device *dev,
>   return 0;
>  }
>  
> +static int gt_legacy_userspace_busy(struct intel_ring_buffer *ring)
> +{
> + int ret;
> +

if (dev_priv->gt_slices.state_default == 1)
return 0;

> + ret = intel_ring_begin(ring, 18);
> + if (ret)
> + return ret;
> +
> + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
> + intel_ring_emit(ring, HSW_GT_SLICE_INFO);
> + intel_ring_emit(ring, SLICE_SEL_BOTH);
> +
> + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
> + intel_ring_emit(ring, MI_PREDICATE_RESULT_2);
> + intel_ring_emit(ring, LOWER_SLICE_ENABLED);
> +
> + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
> + intel_ring_emit(ring, HSW_SLICESHUTDOWN);
> + intel_ring_emit(ring, ~SLICE_SHUTDOWN);
> +
> + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
> + intel_ring_emit(ring, RC_IDLE_MAX_COUNT);
> + intel_ring_emit(ring, CS_IDLE_COUNT_1US);
> +
> + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
> + intel_ring_emit(ring, WAIT_FOR_RC6_EXIT);
> + intel_ring_emit(ring, _MASKED_BIT_ENABLE(WAIT_RC6_EXIT));
> +
> + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
> + intel_ring_emit(ring, RC_IDLE_MAX_COUNT);
> + intel_ring_emit(ring, CS_IDLE_COUNT_5US);
> +
> + intel_ring_advance(ring);
> + return 0;
> +}
> +
> +static bool gt_legacy_userspace(struct intel_ring_buffer *ring,
> + struct drm_i915_gem_execbuffer2 *args)
> +{
> + drm_i915_private_t *dev_priv = ring->dev->dev_private;
> +
> + if (ring->id == BCS)
> + return false;
> +
> + if (!HAS_SLICE_SHUTDOWN(ring->dev))
> + return false;
> +


> + if (dev_priv->gt_slices.state_default == 1)
> + return false;
> +
> + if (dev_priv->gt_slices.legacy_userspace_busy)
> + return false;

Remove these two.

> +
> + return (args->flags & I915_EXEC_USE_PREDICATE) == 0;
> +}
> +
>  static int
>  i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>  struct drm_file *file,
> @@ -999,6 +1055,13 @@ i915_gem_do_execbuffer(struct drm_device *dev, void 
> *data,
>   return -EINVAL;
>   }
>  
> + if (gt_lega

[Intel-gfx] [PULL] drm-intel-fixes

2013-10-25 Thread Daniel Vetter 
Hi Dave,

Just the edp bpp fix from Jani plus the pipe bpp readout code from Ville
to make it work. There's a 3 pipe ivb regression fix pending from me, but
Ville's review convinced me that my first stab is broken.

Cheers, Daniel


The following changes since commit 828c79087cec61eaf4c76bb32c222fbe35ac3930:

  drm/i915: Disable GGTT PTEs on GEN6+ suspend (2013-10-18 15:44:47 +0200)

are available in the git repository at:

  git://people.freedesktop.org/~danvet/drm-intel tags/drm-intel-fixes-2013-10-25

for you to fetch changes up to 52e1e223456e3aa747e9932f95948381f04b3b26:

  drm/i915/dp: workaround BIOS eDP bpp clamping issue (2013-10-21
09:57:02 +0200)


Jani Nikula (1):
  drm/i915/dp: workaround BIOS eDP bpp clamping issue

Ville Syrjälä (1):
  drm/i915: Add support for pipe_bpp readout

 drivers/gpu/drm/i915/intel_ddi.c | 17 +
 drivers/gpu/drm/i915/intel_display.c | 36 
 drivers/gpu/drm/i915/intel_dp.c  | 20 
 3 files changed, 73 insertions(+)

-- 
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] [Dell Inspiron 15R 5521] Can't toggle backlight brightness

2013-10-25 Thread Nicolas Devillers 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Le lun. 14 oct. 2013 11:33:29 CEST, Jani Nikula a écrit :
> On Mon, 14 Oct 2013, Nicolas Devillers  wrote:
>> Unfortunately this seems to cause others unpexected issue like ACPI
>> unstability and crash on suspend/resume.
>> I reported the issue in the following bug
>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1214287
>
> I can imagine the BIOS tripping over itself if you do
> acpi_osi=Linux. Can you reproduce the problems with merely
> acpi_backlight=vendor?
>
> In any case, i915 does exactly the same things with or without
> acpi_backlight=vendor, and the changes are in the ACPI driver. These are
> not i915 bugs.
>
> BR,
> Jani.
>
>
>
>
>>
>>
>> 2013/10/14 Jani Nikula 
>>
>>> On Thu, 10 Oct 2013, Nicolas Devillers 
>>> wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1

 This mail is a follow-up of
 https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1225811

 - - Summary of the problem:

 [Dell Inspiron 15R 5521] (using i915 drv) Can't toggle backlight
>>> brightness

 - - Full description of the problem/report:

 Since Ubuntu Raring at least, it is not possible to setup backlight
 brightness without adding acpi_backlight=vendor into grub boot option.
>>>
>>> That makes the ACPI driver not register its own backlight interface, and
>>> forces use of the i915's intel_backlight interface. Which apparently
>>> works.
>>>
>>> You may find [1] relevant.
>>>
>>> BR,
>>> Jani.
>>>
>>>
>>> [1]
>>> http://mid.gmane.org/1381498066-16011-1-git-send-email-aaron...@intel.com
>>>
>>>
>>>

 - - Kernel version:

 Linux version 3.12.0-031200rc3-generic

 - - Environment

 Description:Ubuntu Saucy Salamander (development branch)
 Release:13.10

 - -  Module information (from /proc/modules):

 ebtable_broute 12731 0 - Live 0x (F)
 bridge 110502 1 ebtable_broute, Live 0x (F)
 stp 12976 1 bridge, Live 0x (F)
 llc 14552 2 bridge,stp, Live 0x (F)
 ebtable_nat 12807 0 - Live 0x (F)
 ebtable_filter 12827 0 - Live 0x (F)
 ebtables 30884 3 ebtable_broute,ebtable_nat,ebtable_filter, Live
 0x (F)
 r8169 67341 0 - Live 0x
 mii 13934 1 r8169, Live 0x (F)
 pci_stub 12622 1 - Live 0x
 vboxpci 23194 0 - Live 0x (OF)
 vboxnetadp 25670 0 - Live 0x (OF)
 vboxnetflt 27613 0 - Live 0x (OF)
 vboxdrv 320455 3 vboxpci,vboxnetadp,vboxnetflt, Live 0x
>>> (OF)
 nfnetlink_queue 18117 0 - Live 0x (F)
 nfnetlink_log 17908 0 - Live 0x (F)
 nfnetlink 14606 2 nfnetlink_queue,nfnetlink_log, Live 0x
>>> (F)
 xt_tcpudp 12884 0 - Live 0x (F)
 nf_conntrack_ipv4 15012 0 - Live 0x (F)
 nf_defrag_ipv4 12729 1 nf_conntrack_ipv4, Live 0x (F)
 xt_conntrack 12760 0 - Live 0x (F)
 nf_conntrack 91736 2 nf_conntrack_ipv4,xt_conntrack, Live
 0x (F)
 ip_tables 27239 0 - Live 0x (F)
 x_tables 34059 4 ebtables,xt_tcpudp,xt_conntrack,ip_tables, Live
 0x (F)
 parport_pc 32701 0 - Live 0x (F)
 ppdev 17671 0 - Live 0x (F)
 bnep 19564 2 - Live 0x
 rfcomm 69070 12 - Live 0x
 binfmt_misc 17468 1 - Live 0x (F)
 nfsd 279192 2 - Live 0x (F)
 auth_rpcgss 54489 1 nfsd, Live 0x (F)
 nfs_acl 12837 1 nfsd, Live 0x (F)
 nfs 180519 0 - Live 0x (F)
 lockd 93583 2 nfsd,nfs, Live 0x (F)
 sunrpc 271025 6 nfsd,auth_rpcgss,nfs_acl,nfs,lockd, Live
 0x (F)
 fscache 58475 1 nfs, Live 0x (F)
 dm_crypt 22728 1 - Live 0x (F)
 nls_iso8859_1 12713 1 - Live 0x (F)
 x86_pkg_temp_thermal 14162 0 - Live 0x
 intel_powerclamp 14705 0 - Live 0x
 kvm_intel 138538 0 - Live 0x (F)
 kvm 431315 1 kvm_intel, Live 0x (F)
 crct10dif_pclmul 14289 0 - Live 0x (F)
 crc32_pclmul 13113 0 - Live 0x (F)
 ghash_clmulni_intel 13259 0 - Live 0x (F)
 aesni_intel 55624 5816 - Live 0x (F)
 aes_x86_64 17131 1 aesni_intel, Live 0x (F)
 lrw 13257 1 aesni_intel, Live 0x (F)
 gf128mul 14951 1 lrw, Live 0x (F)
 glue_helper 13990 1 aesni_intel, Live 0x (F)
 ablk_helper 13597 1 aesni_intel, Live 0x (F)
 cryptd 20329 2909 ghash_clmulni_intel,aesni_

[Intel-gfx] [RFC i-g-t] tests: add runtime_pm

2013-10-25 Thread Paulo Zanoni 
From: Paulo Zanoni 

This test is based on pc8.c. It copies most of the tests from pc8.c,
but it depends on runtime PM status changes (parsed from sysfs)
instead of PC8 residency changes (parsed from the MSR registers).
There's also a test that checks for PC8 residency.

For now, runtime PM and PC8 are different features, so having 2 test
suites makes sense. In the future we'll merge both, so we'll only get
PC8 when runtime PM is enabled, so we'll just kill pc8.c and keep
using runtime_pm.c.

Changes compared to pc8.c:
  - We now look at the runtime PM status instead of PC8 residencies
  - Added more GEM tests (mmap, pread, execbuf, stress tests)
  - Added LPSP and non-LPSP tests
  - Added tests fro sysfs and debugfs files
  - Added a test specifically for PC8

Signed-off-by: Paulo Zanoni 
---
 tests/.gitignore   |1 +
 tests/Makefile.am  |1 +
 tests/runtime_pm.c | 1262 
 3 files changed, 1264 insertions(+)
 create mode 100644 tests/runtime_pm.c

diff --git a/tests/.gitignore b/tests/.gitignore
index 1c97a04..cda6d09 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -105,6 +105,7 @@ prime_nv_pcopy
 prime_nv_test
 prime_self_import
 prime_udl
+runtime_pm
 sysfs_rc6_residency
 sysfs_rps
 testdisplay
diff --git a/tests/Makefile.am b/tests/Makefile.am
index f6ba368..234aaf5 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -59,6 +59,7 @@ TESTS_progs_M = \
$(NOUVEAU_TESTS_M) \
pc8 \
prime_self_import \
+   runtime_pm \
$(NULL)
 
 TESTS_progs = \
diff --git a/tests/runtime_pm.c b/tests/runtime_pm.c
new file mode 100644
index 000..425e743
--- /dev/null
+++ b/tests/runtime_pm.c
@@ -0,0 +1,1262 @@
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Paulo Zanoni 
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "drmtest.h"
+#include "intel_gpu_tools.h"
+#include "igt_display.h"
+
+
+#define MSR_PC8_RES0x630
+#define MSR_PC9_RES0x631
+#define MSR_PC10_RES   0x632
+
+#define POWER_DIR "/sys/devices/pci:00/:00:02.0/power"
+
+#define MAX_CONNECTORS 32
+#define MAX_ENCODERS   32
+#define MAX_CRTCS  16
+
+
+enum runtime_pm_status {
+   RUNTIME_PM_STATUS_ACTIVE,
+   RUNTIME_PM_STATUS_SUSPENDED,
+   RUNTIME_PM_STATUS_SUSPENDING,
+   RUNTIME_PM_STATUS_UNKNOWN,
+};
+
+enum screen_type {
+   SCREEN_TYPE_LPSP,
+   SCREEN_TYPE_NON_LPSP,
+   SCREEN_TYPE_ANY,
+};
+
+/* Stuff used when creating FBs and mode setting. */
+struct mode_set_data {
+   drmModeResPtr res;
+   drmModeConnectorPtr connectors[MAX_CONNECTORS];
+   drmModePropertyBlobPtr edids[MAX_CONNECTORS];
+
+   uint32_t devid;
+};
+
+/* Stuff we query at different times so we can compare. */
+struct compare_data {
+   drmModeResPtr res;
+   drmModeEncoderPtr encoders[MAX_ENCODERS];
+   drmModeConnectorPtr connectors[MAX_CONNECTORS];
+   drmModeCrtcPtr crtcs[MAX_CRTCS];
+   drmModePropertyBlobPtr edids[MAX_CONNECTORS];
+};
+
+
+int drm_fd;
+int status_fd;
+int msr_fd = 0;
+struct mode_set_data ms_data;
+bool quick = false;
+
+
+static uint64_t get_residency(uint32_t type)
+{
+   int rc;
+   uint64_t ret;
+
+   rc = pread(msr_fd, &ret, sizeof(uint64_t), type);
+   igt_assert(rc == sizeof(ret));
+
+   return ret;
+}
+
+static bool pc8_plus_residency_changed(unsigned int timeout_sec)
+{
+   unsigned int i;
+   uint64_t res_pc8, res_pc9, res_pc10;
+   int to_sleep = 100 * 1000;
+
+   res_pc8 = get_residency(MSR_PC8_RES);
+   res_pc9 = get_residency(MSR_PC9_RES);
+   res_pc10 = get_residency(MSR_PC10_RES);
+
+   for (i = 0; i < timeout_sec * 1000 * 1000; i += to_sleep) {
+   i

[Intel-gfx] [PATCH v3 0/4] prepare for multiple power wells

2013-10-25 Thread Imre Deak 
This patchset replaces
"[PATCH 6/6] drm/i915: use power get/put instead of set for power on
after init" from my previous patchset. I addressed the feedback from
Daniel and Paulo (patch 1-3) and included a related cleanup (patch 4).

Imre Deak (4):
  drm/i915: prepare for multiple power wells
  drm/i915: use power get/put instead of set for power on after init
  drm/i915: remove device field from struct power_well
  drm/i915: rename i915_init_power_well to i915_init_power_domains

 drivers/gpu/drm/i915/i915_dma.c  |  10 ++--
 drivers/gpu/drm/i915/i915_drv.c  |   4 +-
 drivers/gpu/drm/i915/i915_drv.h  |  20 +--
 drivers/gpu/drm/i915/intel_display.c |  17 ++
 drivers/gpu/drm/i915/intel_drv.h |   7 ++-
 drivers/gpu/drm/i915/intel_pm.c  | 109 ---
 6 files changed, 92 insertions(+), 75 deletions(-)

-- 
1.8.4

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

[Intel-gfx] [PATCH v3 2/4] drm/i915: use power get/put instead of set for power on after init

2013-10-25 Thread Imre Deak 
Currently we make sure that all power domains are enabled during driver
init and turn off unneded ones only after the first modeset. Similarly
during suspend we enable all power domains, which will remain on through
the following resume until the first modeset.

This logic is supported by intel_set_power_well() in the power domain
framework. It would be nice to simplify the API, so that we only have
get/put functions and make it more explicit on the higher level how this
"power well on during init" logic works. This will make it also easier
if in the future we want to shorten the time the power wells are on.

For this add a new device private flag tracking whether we have the
power wells on because of init/suspend and use only
intel_display_power_get()/put(). As nothing else uses
intel_set_power_well() we can remove it.

This also fixes

commit 6efdf354ddb186c6604d1692075421e8d2c740e9
Author: Imre Deak 
Date:   Wed Oct 16 17:25:52 2013 +0300

drm/i915: enable only the needed power domains during modeset

where removing intel_set_power_well() resulted in not releasing the
reference on the power well that was taken during init and thus leaving
the power well on all the time. Regression reported by Paulo.

v2:
- move the init_power_on flag to the power_domains struct (Daniel)

v3:
- add note about this being a regression fix too (Paulo)

Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/i915_dma.c  |  2 +-
 drivers/gpu/drm/i915/i915_drv.c  |  2 +-
 drivers/gpu/drm/i915/i915_drv.h  |  8 +++-
 drivers/gpu/drm/i915/intel_display.c | 17 +
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c  | 37 +---
 6 files changed, 28 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index fd848ef..b722b35 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1710,7 +1710,7 @@ int i915_driver_unload(struct drm_device *dev)
/* The i915.ko module is still not prepared to be loaded when
 * the power well is not enabled, so just enable it in case
 * we're going to unload/reload. */
-   intel_set_power_well(dev, true);
+   intel_display_set_init_power(dev, true);
i915_remove_power_well(dev);
}
 
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 85ae0dc..770c9f8 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -477,7 +477,7 @@ static int i915_drm_freeze(struct drm_device *dev)
/* We do a lot of poking in a lot of registers, make sure they work
 * properly. */
hsw_disable_package_c8(dev_priv);
-   intel_set_power_well(dev, true);
+   intel_display_set_init_power(dev, true);
 
drm_kms_helper_poll_disable(dev);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7315095..3565db2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -100,6 +100,7 @@ enum intel_display_power_domain {
POWER_DOMAIN_TRANSCODER_C,
POWER_DOMAIN_TRANSCODER_EDP,
POWER_DOMAIN_VGA,
+   POWER_DOMAIN_INIT,
 
POWER_DOMAIN_NUM,
 };
@@ -913,12 +914,17 @@ struct i915_power_well {
struct drm_device *device;
/* power well enable/disable usage count */
int count;
-   int i915_request;
 };
 
 #define I915_MAX_POWER_WELLS 1
 
 struct i915_power_domains {
+   /*
+* Power wells needed for initialization at driver init and suspend
+* time are on. They are kept on until after the first modeset.
+*/
+   bool init_power_on;
+
struct mutex lock;
struct i915_power_well power_wells[I915_MAX_POWER_WELLS];
 };
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 3e79a2a..0c2e83c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6573,6 +6573,21 @@ static unsigned long get_pipe_power_domains(struct 
drm_device *dev,
return mask;
 }
 
+void intel_display_set_init_power(struct drm_device *dev, bool enable)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+
+   if (dev_priv->power_domains.init_power_on == enable)
+   return;
+
+   if (enable)
+   intel_display_power_get(dev, POWER_DOMAIN_INIT);
+   else
+   intel_display_power_put(dev, POWER_DOMAIN_INIT);
+
+   dev_priv->power_domains.init_power_on = enable;
+}
+
 static void modeset_update_power_wells(struct drm_device *dev)
 {
unsigned long pipe_domains[I915_MAX_PIPES] = { 0, };
@@ -6604,6 +6619,8 @@ static void modeset_update_power_wells(struct drm_device 
*dev)
 
crtc->enabled_power_domains = pipe_domains[crtc->pipe];
}
+
+   intel_display_set_init_power(dev, false);

[Intel-gfx] [PATCH v3 1/4] drm/i915: prepare for multiple power wells

2013-10-25 Thread Imre Deak 
In the future we'll need to support multiple power wells, so prepare for
that here. Create a new power domains struct which contains all
power domain/well specific fields. Since we'll have one lock protecting
all power wells, move power_well->lock to the new struct too.

No functional change.

Signed-off-by: Imre Deak 
Reviewed-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/i915_drv.h | 11 ++---
 drivers/gpu/drm/i915/intel_pm.c | 55 +
 2 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 80957ca..7315095 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -911,12 +911,18 @@ struct intel_ilk_power_mgmt {
 /* Power well structure for haswell */
 struct i915_power_well {
struct drm_device *device;
-   struct mutex lock;
/* power well enable/disable usage count */
int count;
int i915_request;
 };
 
+#define I915_MAX_POWER_WELLS 1
+
+struct i915_power_domains {
+   struct mutex lock;
+   struct i915_power_well power_wells[I915_MAX_POWER_WELLS];
+};
+
 struct i915_dri1_state {
unsigned allow_batchbuffer : 1;
u32 __iomem *gfx_hws_cpu_addr;
@@ -1412,8 +1418,7 @@ typedef struct drm_i915_private {
 * mchdev_lock in intel_pm.c */
struct intel_ilk_power_mgmt ips;
 
-   /* Haswell power well */
-   struct i915_power_well power_well;
+   struct i915_power_domains power_domains;
 
struct i915_psr psr;
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3e140ab..09ca6f9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5625,7 +5625,7 @@ void intel_display_power_get(struct drm_device *dev,
 enum intel_display_power_domain domain)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
-   struct i915_power_well *power_well = &dev_priv->power_well;
+   struct i915_power_domains *power_domains;
 
if (!HAS_POWER_WELL(dev))
return;
@@ -5633,16 +5633,18 @@ void intel_display_power_get(struct drm_device *dev,
if (is_always_on_power_domain(dev, domain))
return;
 
-   mutex_lock(&power_well->lock);
-   __intel_power_well_get(power_well);
-   mutex_unlock(&power_well->lock);
+   power_domains = &dev_priv->power_domains;
+
+   mutex_lock(&power_domains->lock);
+   __intel_power_well_get(&power_domains->power_wells[0]);
+   mutex_unlock(&power_domains->lock);
 }
 
 void intel_display_power_put(struct drm_device *dev,
 enum intel_display_power_domain domain)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
-   struct i915_power_well *power_well = &dev_priv->power_well;
+   struct i915_power_domains *power_domains;
 
if (!HAS_POWER_WELL(dev))
return;
@@ -5650,12 +5652,14 @@ void intel_display_power_put(struct drm_device *dev,
if (is_always_on_power_domain(dev, domain))
return;
 
-   mutex_lock(&power_well->lock);
-   __intel_power_well_put(power_well);
-   mutex_unlock(&power_well->lock);
+   power_domains = &dev_priv->power_domains;
+
+   mutex_lock(&power_domains->lock);
+   __intel_power_well_put(&power_domains->power_wells[0]);
+   mutex_unlock(&power_domains->lock);
 }
 
-static struct i915_power_well *hsw_pwr;
+static struct i915_power_domains *hsw_pwr;
 
 /* Display audio driver power well request */
 void i915_request_power_well(void)
@@ -5664,7 +5668,7 @@ void i915_request_power_well(void)
return;
 
mutex_lock(&hsw_pwr->lock);
-   __intel_power_well_get(hsw_pwr);
+   __intel_power_well_get(&hsw_pwr->power_wells[0]);
mutex_unlock(&hsw_pwr->lock);
 }
 EXPORT_SYMBOL_GPL(i915_request_power_well);
@@ -5676,7 +5680,7 @@ void i915_release_power_well(void)
return;
 
mutex_lock(&hsw_pwr->lock);
-   __intel_power_well_put(hsw_pwr);
+   __intel_power_well_put(&hsw_pwr->power_wells[0]);
mutex_unlock(&hsw_pwr->lock);
 }
 EXPORT_SYMBOL_GPL(i915_release_power_well);
@@ -5684,12 +5688,15 @@ EXPORT_SYMBOL_GPL(i915_release_power_well);
 int i915_init_power_well(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
+   struct i915_power_domains *power_domains = &dev_priv->power_domains;
+   struct i915_power_well *power_well;
 
-   hsw_pwr = &dev_priv->power_well;
+   mutex_init(&power_domains->lock);
+   hsw_pwr = power_domains;
 
-   hsw_pwr->device = dev;
-   mutex_init(&hsw_pwr->lock);
-   hsw_pwr->count = 0;
+   power_well = &power_domains->power_wells[0];
+   power_well->device = dev;
+   power_well->count = 0;
 
return 0;
 }
@@ -5702,7 +5709,8 @@ void i915_remove_power_well(struct drm_device *dev)
 void intel_

[Intel-gfx] [PATCH v3 3/4] drm/i915: remove device field from struct power_well

2013-10-25 Thread Imre Deak 
The only real need for this field was in
i915_{request,release}_power_well, but there we can get at it by a
container_of magic. Also since in the future we'll have multiple power
wells each with its own power_well struct it makes sense to remove the
field from there where it'd be just redundancy.

Suggested-by: Paulo Zanoni 
Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/i915_drv.h |  1 -
 drivers/gpu/drm/i915/intel_pm.c | 29 -
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3565db2..2731fbb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -911,7 +911,6 @@ struct intel_ilk_power_mgmt {
 
 /* Power well structure for haswell */
 struct i915_power_well {
-   struct drm_device *device;
/* power well enable/disable usage count */
int count;
 };
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4c38e28..4f84a4b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5608,17 +5608,19 @@ static void __intel_set_power_well(struct drm_device 
*dev, bool enable)
}
 }
 
-static void __intel_power_well_get(struct i915_power_well *power_well)
+static void __intel_power_well_get(struct drm_device *dev,
+  struct i915_power_well *power_well)
 {
if (!power_well->count++)
-   __intel_set_power_well(power_well->device, true);
+   __intel_set_power_well(dev, true);
 }
 
-static void __intel_power_well_put(struct i915_power_well *power_well)
+static void __intel_power_well_put(struct drm_device *dev,
+  struct i915_power_well *power_well)
 {
WARN_ON(!power_well->count);
if (!--power_well->count)
-   __intel_set_power_well(power_well->device, false);
+   __intel_set_power_well(dev, false);
 }
 
 void intel_display_power_get(struct drm_device *dev,
@@ -5636,7 +5638,7 @@ void intel_display_power_get(struct drm_device *dev,
power_domains = &dev_priv->power_domains;
 
mutex_lock(&power_domains->lock);
-   __intel_power_well_get(&power_domains->power_wells[0]);
+   __intel_power_well_get(dev, &power_domains->power_wells[0]);
mutex_unlock(&power_domains->lock);
 }
 
@@ -5655,7 +5657,7 @@ void intel_display_power_put(struct drm_device *dev,
power_domains = &dev_priv->power_domains;
 
mutex_lock(&power_domains->lock);
-   __intel_power_well_put(&power_domains->power_wells[0]);
+   __intel_power_well_put(dev, &power_domains->power_wells[0]);
mutex_unlock(&power_domains->lock);
 }
 
@@ -5664,11 +5666,16 @@ static struct i915_power_domains *hsw_pwr;
 /* Display audio driver power well request */
 void i915_request_power_well(void)
 {
+   struct drm_i915_private *dev_priv;
+
if (WARN_ON(!hsw_pwr))
return;
 
+   dev_priv = container_of(hsw_pwr, struct drm_i915_private,
+   power_domains);
+
mutex_lock(&hsw_pwr->lock);
-   __intel_power_well_get(&hsw_pwr->power_wells[0]);
+   __intel_power_well_get(dev_priv->dev, &hsw_pwr->power_wells[0]);
mutex_unlock(&hsw_pwr->lock);
 }
 EXPORT_SYMBOL_GPL(i915_request_power_well);
@@ -5676,11 +5683,16 @@ EXPORT_SYMBOL_GPL(i915_request_power_well);
 /* Display audio driver power well release */
 void i915_release_power_well(void)
 {
+   struct drm_i915_private *dev_priv;
+
if (WARN_ON(!hsw_pwr))
return;
 
+   dev_priv = container_of(hsw_pwr, struct drm_i915_private,
+   power_domains);
+
mutex_lock(&hsw_pwr->lock);
-   __intel_power_well_put(&hsw_pwr->power_wells[0]);
+   __intel_power_well_put(dev_priv->dev, &hsw_pwr->power_wells[0]);
mutex_unlock(&hsw_pwr->lock);
 }
 EXPORT_SYMBOL_GPL(i915_release_power_well);
@@ -5695,7 +5707,6 @@ int i915_init_power_well(struct drm_device *dev)
hsw_pwr = power_domains;
 
power_well = &power_domains->power_wells[0];
-   power_well->device = dev;
power_well->count = 0;
 
return 0;
-- 
1.8.4

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

[Intel-gfx] [PATCH v3 4/4] drm/i915: rename i915_init_power_well to i915_init_power_domains

2013-10-25 Thread Imre Deak 
Similarly rename the other related functions in the power domain
interface.

Higher level driver code calling these functions knows only about power
domains, not the underlying power wells which may be different on
different platforms. Also these functions really init/cleanup/resume
power domains and only through that all related power wells, so rename
them accordingly.

Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/i915_dma.c  |  8 
 drivers/gpu/drm/i915/i915_drv.c  |  2 +-
 drivers/gpu/drm/i915/intel_drv.h |  6 +++---
 drivers/gpu/drm/i915/intel_pm.c  | 10 +-
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index b722b35..75f02e4 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1311,7 +1311,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
if (ret)
goto cleanup_gem_stolen;
 
-   intel_init_power_well(dev);
+   intel_init_power_domains(dev);
 
/* Important: The output setup functions called by modeset_init need
 * working irqs for e.g. gmbus and dp aux transfers. */
@@ -1640,7 +1640,7 @@ int i915_driver_load(struct drm_device *dev, unsigned 
long flags)
}
 
if (HAS_POWER_WELL(dev))
-   i915_init_power_well(dev);
+   i915_init_power_domains(dev);
 
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
ret = i915_load_modeset_init(dev);
@@ -1668,7 +1668,7 @@ int i915_driver_load(struct drm_device *dev, unsigned 
long flags)
 
 out_power_well:
if (HAS_POWER_WELL(dev))
-   i915_remove_power_well(dev);
+   i915_remove_power_domains(dev);
drm_vblank_cleanup(dev);
 out_gem_unload:
if (dev_priv->mm.inactive_shrinker.scan_objects)
@@ -1711,7 +1711,7 @@ int i915_driver_unload(struct drm_device *dev)
 * the power well is not enabled, so just enable it in case
 * we're going to unload/reload. */
intel_display_set_init_power(dev, true);
-   i915_remove_power_well(dev);
+   i915_remove_power_domains(dev);
}
 
i915_teardown_sysfs(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 770c9f8..e4474c6 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -597,7 +597,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool 
restore_gtt_mappings)
mutex_unlock(&dev->struct_mutex);
}
 
-   intel_init_power_well(dev);
+   intel_init_power_domains(dev);
 
i915_restore_state(dev);
intel_opregion_setup(dev);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bf4394a..ae6d362 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -823,15 +823,15 @@ bool intel_fbc_enabled(struct drm_device *dev);
 void intel_update_fbc(struct drm_device *dev);
 void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
 void intel_gpu_ips_teardown(void);
-int i915_init_power_well(struct drm_device *dev);
-void i915_remove_power_well(struct drm_device *dev);
+int i915_init_power_domains(struct drm_device *dev);
+void i915_remove_power_domains(struct drm_device *dev);
 bool intel_display_power_enabled(struct drm_device *dev,
 enum intel_display_power_domain domain);
 void intel_display_power_get(struct drm_device *dev,
 enum intel_display_power_domain domain);
 void intel_display_power_put(struct drm_device *dev,
 enum intel_display_power_domain domain);
-void intel_init_power_well(struct drm_device *dev);
+void intel_init_power_domains(struct drm_device *dev);
 void intel_set_power_well(struct drm_device *dev, bool enable);
 void intel_enable_gt_powersave(struct drm_device *dev);
 void intel_disable_gt_powersave(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4f84a4b..9afec23 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5697,7 +5697,7 @@ void i915_release_power_well(void)
 }
 EXPORT_SYMBOL_GPL(i915_release_power_well);
 
-int i915_init_power_well(struct drm_device *dev)
+int i915_init_power_domains(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
struct i915_power_domains *power_domains = &dev_priv->power_domains;
@@ -5712,12 +5712,12 @@ int i915_init_power_well(struct drm_device *dev)
return 0;
 }
 
-void i915_remove_power_well(struct drm_device *dev)
+void i915_remove_power_domains(struct drm_device *dev)
 {
hsw_pwr = NULL;
 }
 
-static void intel_resume_power_well(struct drm_device *dev)
+static void intel_resume_power_domains(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;

Re: [Intel-gfx] [v3.10][v3.11][v3.12][Regression][PATCH 1/1] Revert "Revert "drm/i915: revert eDP bpp clamping code changes""

2013-10-25 Thread Joseph Salisbury 
On 10/16/2013 05:02 PM, Daniel Vetter wrote:
> On Wed, Oct 16, 2013 at 04:34:57PM -0400, Joseph Salisbury wrote:
>> BugLink: http://bugs.launchpad.net/bugs/1195483
>>
>> This reverts commit 657445fe8660100ad174600ebfa61536392b7624.
>>
>> Signed-off-by: Joseph Salisbury 
>> Cc: Daniel Vetter 
>> Cc: Paulo Zanoni 
>> Cc: David Airlie 
>> Cc: sta...@vger.kernel.org
> It's by far not that simple. Jani is working on both the underlying bug
> and a better w/a. See
>
> https://bugzilla.kernel.org/show_bug.cgi?id=59841
>
> for the full story in its entire glory.
>
> Cheers, Daniel

Thanks for the feedback, Daniel.  Is there an estimate on what mainline
release might contain Jani's work?

Thanks again,

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

[Intel-gfx] Tizen IVI EFI 3.0 graphics failure with FSP/OTM on Bayley Bay

2013-10-25 Thread Wang, Stephanie 
Jesse, hey
I work for ISG FSP/OTM boot loader team under Khadker Islam. I've recently run 
into an issue that blocks FSP/OTM from supporting Tizen IVI 3.0 running Bay 
Trail B3 silicon on Bayley Bay board.
Log file with the failure is attached. The failure is very consistent, happens 
every time with Bay Trail B3 silicon running FSP/OTM boot loader. The boot 
image was generated from Tizen IVI 3.0 EFI image posted at 
http://download.tizen.org/releases/milestone/tizen/ivi/latest/images/ivi-release-efi-i586/.
 The original image from the sever booted with EFI BIOS on the same board. Our 
FSP/OTM is legacy boot loader without EFI support. Failure seems to be with the 
graphics driver. The messages from the failure log file are shown on both 
serial console and local console via VGA. I need some help to understand if the 
driver under efi OS only works with efi BIOS? Does it need some efi run time 
services from the BIOS? Or does it need certain POST set up from the BIOS? 
Also, is there a legacy version of Tizen IVI 3.0 ? if yes, where do I get this? 
Any help will be greatly appreciated!
Stephanie
Intel(R) Boot Loader Development Kit
[0.00] Initializing cgroup subsys cpusetights reserved.
[0.00] Initializing cgroup subsys cpu
[0.00] Initializing cgroup subsys cpuacct
[0.00] Linux version 3.11.0-09202013 (root@jessie-vlv-bs) (gcc version 
4.6.3 20120306 (Red Hat 4.6.3-2) (GCC) ) #8 SMP PREEMPT Tue Oct 1 21:11:32 MYT 
2013
[0.00] e820: BIOS-provided physical RAM map:
[0.00] BIOS-e820: [mem 0x-0x00097fff] usable
[0.00] BIOS-e820: [mem 0x00098000-0x0009] reserved
[0.00] BIOS-e820: [mem 0x000e-0x000f] reserved
[0.00] BIOS-e820: [mem 0x0010-0x5fff] usable
[0.00] BIOS-e820: [mem 0x6000-0x600f] reserved
[0.00] BIOS-e820: [mem 0x6010-0x7b9e] usable
[0.00] BIOS-e820: [mem 0x7b9f-0x7b9ff5ff] ACPI data
[0.00] BIOS-e820: [mem 0x7b9ff600-0x7fff] reserved
[0.00] BIOS-e820: [mem 0xe000-0xefff] reserved
[0.00] BIOS-e820: [mem 0xfec0-0xfec00fff] reserved
[0.00] BIOS-e820: [mem 0xfed1c000-0xfed1] reserved
[0.00] BIOS-e820: [mem 0xfee0-0xfee00fff] 
reservedex[0.00] BIOS-e820: [mem 0xff80-0x] 
unusable
[0.00] BIOS-e820: [mem 0x0001-0x00017fff] usable
[0.00] Notice: NX (Execute Disable) protection cannot be enabled: 
non-PAE kernel!
[0.00] DMI not present or invalid.a_load->kernel_execute)
[0.00] e820: last_pfn = 0x7b9f0 max_arch_pfn = 0x10
[0.00] x86 PAT enabled: cpu 0, old 0x7040600070406, new 0x7010600070106
[0.00] found SMP MP-table at [mem 0x000fd840-0x000fd84f] mapped at 
[c00fd840]
[0.00] init_memory_mapping: [mem 0x-0x000f]
[0.00] init_memory_mapping: [mem 0x3700-0x373f]
[0.00] init_memory_mapping: [mem 0x3000-0x36ff]
[0.00] init_memory_mapping: [mem 0x0010-0x2fff]
[0.00] init_memory_mapping: [mem 0x3740-0x377fdfff]
[0.00] ACPI: RSDP 000e0110 00024 (v02 INTEL )
[0.00] ACPI: XSDT 7b9f7c90 0004C (v01 INTEL     
)
[0.00] ACPI: FACP 7b9f7b90 000F4 (v03 INTEL     
)
[0.00] ACPI: DSDT 7b9f7d10 078E4 (v02  INTEL  VLV-SOC  INTL 
20120913)
[0.00] ACPI: FACS 7b9ff600 00040
[0.00] ACPI: APIC 7b9f5b60 0008A (v01 INTEL     
)
[0.00] ACPI: MCFG 7b9f5b20 0003C (v01 INTEL  MCFG   
)
[0.00] ACPI: HPET 7b9f5ae0 00038 (v01 INTEL     
)
[0.00] ACPI: SSDT 7b9f5370 0076B (v01  PmRefCpuPm 3000 INTL 
20120913)
[0.00] ACPI: no DMI BIOS year, acpi=force is required to enable ACPI
[0.00] ACPI: acpi=force override
[0.00] 1089MB HIGHMEM available.
[0.00] 887MB LOWMEM available.
[0.00]   mapped low ram: 0 - 377fe000
[0.00]   low ram: 0 - 377fe000
[0.00] Zone ranges:
[0.00]   DMA  [mem 0x1000-0x00ff]
[0.00]   Normal   [mem 0x0100-0x377fdfff]
[0.00]   HighMem  [mem 0x377fe000-0x7b9e]
[0.00] Movable zone start for each node
[0.00] Early memory node ranges
[0.00]   node   0: [mem 0x1000-0x00097fff]
[0.00]   node   0: [mem 0x0010-0x5fff]
[0.00]   node   0: [mem 0x6010-0x7b9e]
[0.00] Using APIC driver default
[0.00] ACPI: PM-Timer IO Port: 0x408
[0.00] ACPI: LAPIC (acpi_id[0x01] lapic_id[0x00] enabled)
[0.00] ACPI: LAPIC (acpi_id[0x02] lapic_id[0x02] enabled)
[0.0

Re: [Intel-gfx] [PATCH 3/4] drm/i915: WaFbcDisableDpfcrClockGating only with fbc

2013-10-25 Thread Paulo Zanoni 
2013/10/24 Ben Widawsky :
> We were turning this on for ILK regardless of whether or not we use FBC.
> We can save the slightest amount of power if we don't disable it when
> not using FBC.

Finally someone did what I requested months ago:
http://lists.freedesktop.org/archives/intel-gfx/2013-June/028906.html
:)


>
> The workaround should be bit 8 for ILK. Notice it is 1 bit difference
> from SNB. This is actually DPFCR unit as we've defined it.

Ok, so we have bits 8 and 9. Judging by the register names, I would
say bit 9 is WaFbcDisableDpfcClockGating (without 'r') and bit 8 is
the ironlake-only WaFbcDisableDpfcrClockGating. Is that right?


>
> Signed-off-by: Ben Widawsky 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 686699c..bbcf100 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -238,6 +238,11 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, 
> unsigned long interval)
>SNB_CPU_FENCE_ENABLE | obj->fence_reg);
> I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
> sandybridge_blit_fbc_update(dev);
> +   } else {
> +   /* WaFbcDisableDpfcClockGating:ilk */

If you agree with me on the question above, then the WA name here is
missing an 'r' char.


> +   I915_WRITE(ILK_DSPCLK_GATE_D,
> +  I915_READ(ILK_DSPCLK_GATE_D) |
> +  ILK_DPFCRUNIT_CLOCK_GATE_DISABLE);
> }
>
> DRM_DEBUG_KMS("enabled fbc on plane %c\n", 
> plane_name(intel_crtc->plane));
> @@ -254,6 +259,12 @@ static void ironlake_disable_fbc(struct drm_device *dev)
> dpfc_ctl &= ~DPFC_CTL_EN;
> I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl);
>
> +   if (IS_GEN5(dev))
> +   /* WaFbcDisableDpfcClockGating:ilk */

Same here.


> +   I915_WRITE(ILK_DSPCLK_GATE_D,
> +  I915_READ(ILK_DSPCLK_GATE_D) &
> +  ~ILK_DPFCRUNIT_CLOCK_GATE_DISABLE);
> +
> DRM_DEBUG_KMS("disabled FBC\n");
> }
>  }
> @@ -4932,9 +4943,9 @@ static void ironlake_init_clock_gating(struct 
> drm_device *dev)
>
> /*
>  * Required for FBC
> -* WaFbcDisableDpfcClockGating:ilk
> +* WaFbcDisableDpfcClockGating:snb

The ":snb" part is certainly wrong since this function doesn't run on SNB.

The actual code (excluding comments) looks correct.


>  */
> -   dspclk_gate |= ILK_DPFCRUNIT_CLOCK_GATE_DISABLE |
> +   dspclk_gate |=
>ILK_DPFCUNIT_CLOCK_GATE_DISABLE |
>ILK_DPFDUNIT_CLOCK_GATE_ENABLE;
>
> --
> 1.8.4.1
>
> ___
> 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: WaFbcDisableDpfcClockGating only with fbc

2013-10-25 Thread Paulo Zanoni 
2013/10/24 Ben Widawsky :
> We were turning this on for SNB regardless of whether or not we use FBC.
> We can save the slightest amount of power if we don't disable it when
> not using FBC.
>
> The workaround should be bit 9 for SNB.

First, see comment in patch 3. So you're removing the WA on ILK and
applying it for SNB-only. Since the spec doesn't say "SNB only", I
guess we need this WA on both gens.

>
> Signed-off-by: Ben Widawsky 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 25 +++--
>  1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index bbcf100..4ebbe65 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -237,6 +237,12 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, 
> unsigned long interval)
> I915_WRITE(SNB_DPFC_CTL_SA,
>SNB_CPU_FENCE_ENABLE | obj->fence_reg);
> I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
> +
> +   /* WaFbcDisableDpfcClockGating:snb */
> +   I915_WRITE(ILK_DSPCLK_GATE_D,
> +  I915_READ(ILK_DSPCLK_GATE_D) |
> +  ILK_DPFCUNIT_CLOCK_GATE_DISABLE);
> +
> sandybridge_blit_fbc_update(dev);
> } else {
> /* WaFbcDisableDpfcClockGating:ilk */
> @@ -259,7 +265,12 @@ static void ironlake_disable_fbc(struct drm_device *dev)
> dpfc_ctl &= ~DPFC_CTL_EN;
> I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl);
>
> -   if (IS_GEN5(dev))
> +   if (IS_GEN6(dev))
> +   /* WaFbcDisableDpfcClockGating:snb */
> +   I915_WRITE(ILK_DSPCLK_GATE_D,
> +  I915_READ(ILK_DSPCLK_GATE_D) &
> +  ~ILK_DPFCUNIT_CLOCK_GATE_DISABLE);
> +   else if (IS_GEN5(dev))
> /* WaFbcDisableDpfcClockGating:ilk */
> I915_WRITE(ILK_DSPCLK_GATE_D,
>I915_READ(ILK_DSPCLK_GATE_D) &
> @@ -4939,15 +4950,9 @@ static void g4x_disable_trickle_feed(struct drm_device 
> *dev)
>  static void ironlake_init_clock_gating(struct drm_device *dev)
>  {
> struct drm_i915_private *dev_priv = dev->dev_private;
> -   uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE;
> -
> -   /*
> -* Required for FBC
> -* WaFbcDisableDpfcClockGating:snb
> -*/
> -   dspclk_gate |=
> -  ILK_DPFCUNIT_CLOCK_GATE_DISABLE |
> -  ILK_DPFDUNIT_CLOCK_GATE_ENABLE;
> +   uint32_t dspclk_gate =
> +   ILK_VRHUNIT_CLOCK_GATE_DISABLE |
> +   ILK_DPFDUNIT_CLOCK_GATE_ENABLE;
>
> I915_WRITE(PCH_3DCGDIS0,
>MARIUNIT_CLOCK_GATE_DISABLE |
> --
> 1.8.4.1
>
> ___
> 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 1/4] drm/i915: Remove WaFbcDisableDpfcClockGating on IVB

2013-10-25 Thread Paulo Zanoni 
2013/10/24 Ben Widawsky :
> Production IVB does not need it. I confirmed this with Art.
>
> Signed-off-by: Ben Widawsky 

Reviewed-by: Paulo Zanoni 

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 10 --
>  1 file changed, 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d4dd543..33ad028 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -254,12 +254,6 @@ static void ironlake_disable_fbc(struct drm_device *dev)
> dpfc_ctl &= ~DPFC_CTL_EN;
> I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl);
>
> -   if (IS_IVYBRIDGE(dev))
> -   /* WaFbcDisableDpfcClockGating:ivb */
> -   I915_WRITE(ILK_DSPCLK_GATE_D,
> -  I915_READ(ILK_DSPCLK_GATE_D) &
> -  ~ILK_DPFCUNIT_CLOCK_GATE_DISABLE);
> -
> if (IS_HASWELL(dev))
> /* WaFbcDisableDpfcClockGating:hsw */
> I915_WRITE(HSW_CLKGATE_DISABLE_PART_1,
> @@ -295,10 +289,6 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, 
> unsigned long interval)
> if (IS_IVYBRIDGE(dev)) {
> /* WaFbcAsynchFlipDisableFbcQueue:ivb */
> I915_WRITE(ILK_DISPLAY_CHICKEN1, ILK_FBCQ_DIS);
> -   /* WaFbcDisableDpfcClockGating:ivb */
> -   I915_WRITE(ILK_DSPCLK_GATE_D,
> -  I915_READ(ILK_DSPCLK_GATE_D) |
> -  ILK_DPFCUNIT_CLOCK_GATE_DISABLE);
> } else {
> /* WaFbcAsynchFlipDisableFbcQueue:hsw */
> I915_WRITE(HSW_PIPE_SLICE_CHICKEN_1(intel_crtc->pipe),
> --
> 1.8.4.1
>
> ___
> 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/4] drm/i915: Remove WaFbcDisableDpfcClockGating on HSW

2013-10-25 Thread Paulo Zanoni 
2013/10/24 Ben Widawsky :
> Production HSW does not need it. I confirmed this with Art.
>
> Signed-off-by: Ben Widawsky 

I just hope these things don't start uncovering bugs :)

Reviewed-by: Paulo Zanoni 

> ---
>  drivers/gpu/drm/i915/i915_reg.h |  3 ---
>  drivers/gpu/drm/i915/intel_pm.c | 10 --
>  2 files changed, 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 6c98238..6799d53 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1110,9 +1110,6 @@
>  _HSW_PIPE_SLICE_CHICKEN_1_A, + \
>  _HSW_PIPE_SLICE_CHICKEN_1_B)
>
> -#define HSW_CLKGATE_DISABLE_PART_1 0x46500
> -#define   HSW_DPFC_GATING_DISABLE  (1<<23)
> -
>  /*
>   * GPIO regs
>   */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 33ad028..686699c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -254,12 +254,6 @@ static void ironlake_disable_fbc(struct drm_device *dev)
> dpfc_ctl &= ~DPFC_CTL_EN;
> I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl);
>
> -   if (IS_HASWELL(dev))
> -   /* WaFbcDisableDpfcClockGating:hsw */
> -   I915_WRITE(HSW_CLKGATE_DISABLE_PART_1,
> -  I915_READ(HSW_CLKGATE_DISABLE_PART_1) &
> -  ~HSW_DPFC_GATING_DISABLE);
> -
> DRM_DEBUG_KMS("disabled FBC\n");
> }
>  }
> @@ -293,10 +287,6 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, 
> unsigned long interval)
> /* WaFbcAsynchFlipDisableFbcQueue:hsw */
> I915_WRITE(HSW_PIPE_SLICE_CHICKEN_1(intel_crtc->pipe),
>HSW_BYPASS_FBC_QUEUE);
> -   /* WaFbcDisableDpfcClockGating:hsw */
> -   I915_WRITE(HSW_CLKGATE_DISABLE_PART_1,
> -  I915_READ(HSW_CLKGATE_DISABLE_PART_1) |
> -  HSW_DPFC_GATING_DISABLE);
> }
>
> I915_WRITE(SNB_DPFC_CTL_SA,
> --
> 1.8.4.1
>
> ___
> 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 v3 2/4] drm/i915: use power get/put instead of set for power on after init

2013-10-25 Thread Paulo Zanoni 
2013/10/25 Imre Deak :
> Currently we make sure that all power domains are enabled during driver
> init and turn off unneded ones only after the first modeset. Similarly
> during suspend we enable all power domains, which will remain on through
> the following resume until the first modeset.
>
> This logic is supported by intel_set_power_well() in the power domain
> framework. It would be nice to simplify the API, so that we only have
> get/put functions and make it more explicit on the higher level how this
> "power well on during init" logic works. This will make it also easier
> if in the future we want to shorten the time the power wells are on.
>
> For this add a new device private flag tracking whether we have the
> power wells on because of init/suspend and use only
> intel_display_power_get()/put(). As nothing else uses
> intel_set_power_well() we can remove it.
>
> This also fixes
>
> commit 6efdf354ddb186c6604d1692075421e8d2c740e9
> Author: Imre Deak 
> Date:   Wed Oct 16 17:25:52 2013 +0300
>
> drm/i915: enable only the needed power domains during modeset
>
> where removing intel_set_power_well() resulted in not releasing the
> reference on the power well that was taken during init and thus leaving
> the power well on all the time. Regression reported by Paulo.
>
> v2:
> - move the init_power_on flag to the power_domains struct (Daniel)
>
> v3:
> - add note about this being a regression fix too (Paulo)

I didn't closely follow the first 5 patches, but this one makes sense.

Reviewed-by: Paulo Zanoni 

>
> Signed-off-by: Imre Deak 
> ---
>  drivers/gpu/drm/i915/i915_dma.c  |  2 +-
>  drivers/gpu/drm/i915/i915_drv.c  |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h  |  8 +++-
>  drivers/gpu/drm/i915/intel_display.c | 17 +
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  drivers/gpu/drm/i915/intel_pm.c  | 37 
> +---
>  6 files changed, 28 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index fd848ef..b722b35 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1710,7 +1710,7 @@ int i915_driver_unload(struct drm_device *dev)
> /* The i915.ko module is still not prepared to be loaded when
>  * the power well is not enabled, so just enable it in case
>  * we're going to unload/reload. */
> -   intel_set_power_well(dev, true);
> +   intel_display_set_init_power(dev, true);
> i915_remove_power_well(dev);
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 85ae0dc..770c9f8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -477,7 +477,7 @@ static int i915_drm_freeze(struct drm_device *dev)
> /* We do a lot of poking in a lot of registers, make sure they work
>  * properly. */
> hsw_disable_package_c8(dev_priv);
> -   intel_set_power_well(dev, true);
> +   intel_display_set_init_power(dev, true);
>
> drm_kms_helper_poll_disable(dev);
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7315095..3565db2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -100,6 +100,7 @@ enum intel_display_power_domain {
> POWER_DOMAIN_TRANSCODER_C,
> POWER_DOMAIN_TRANSCODER_EDP,
> POWER_DOMAIN_VGA,
> +   POWER_DOMAIN_INIT,
>
> POWER_DOMAIN_NUM,
>  };
> @@ -913,12 +914,17 @@ struct i915_power_well {
> struct drm_device *device;
> /* power well enable/disable usage count */
> int count;
> -   int i915_request;
>  };
>
>  #define I915_MAX_POWER_WELLS 1
>
>  struct i915_power_domains {
> +   /*
> +* Power wells needed for initialization at driver init and suspend
> +* time are on. They are kept on until after the first modeset.
> +*/
> +   bool init_power_on;
> +
> struct mutex lock;
> struct i915_power_well power_wells[I915_MAX_POWER_WELLS];
>  };
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 3e79a2a..0c2e83c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6573,6 +6573,21 @@ static unsigned long get_pipe_power_domains(struct 
> drm_device *dev,
> return mask;
>  }
>
> +void intel_display_set_init_power(struct drm_device *dev, bool enable)
> +{
> +   struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +   if (dev_priv->power_domains.init_power_on == enable)
> +   return;
> +
> +   if (enable)
> +   intel_display_power_get(dev, POWER_DOMAIN_INIT);
> +   else
> +   intel_display_power_put(dev, POWER_DOMAIN_INIT);
> +
> +   dev_priv->power_domains.init_power_on = enable;
> +}

Re: [Intel-gfx] [PATCH v3 3/4] drm/i915: remove device field from struct power_well

2013-10-25 Thread Paulo Zanoni 
2013/10/25 Imre Deak :
> The only real need for this field was in
> i915_{request,release}_power_well, but there we can get at it by a
> container_of magic. Also since in the future we'll have multiple power
> wells each with its own power_well struct it makes sense to remove the
> field from there where it'd be just redundancy.
>
> Suggested-by: Paulo Zanoni 

My original idea was to just move it from i915_power_well to
i915_power_domains, so hsw_pwr (which is the new external static
thing) would still have a pointer to our driver. This way we wouldn't
need the container_of magic. But your solution works too, and saves
4/8 bytes :)

Reviewed-by: Paulo Zanoni 

> Signed-off-by: Imre Deak 
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  1 -
>  drivers/gpu/drm/i915/intel_pm.c | 29 -
>  2 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3565db2..2731fbb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -911,7 +911,6 @@ struct intel_ilk_power_mgmt {
>
>  /* Power well structure for haswell */
>  struct i915_power_well {
> -   struct drm_device *device;
> /* power well enable/disable usage count */
> int count;
>  };
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 4c38e28..4f84a4b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5608,17 +5608,19 @@ static void __intel_set_power_well(struct drm_device 
> *dev, bool enable)
> }
>  }
>
> -static void __intel_power_well_get(struct i915_power_well *power_well)
> +static void __intel_power_well_get(struct drm_device *dev,
> +  struct i915_power_well *power_well)
>  {
> if (!power_well->count++)
> -   __intel_set_power_well(power_well->device, true);
> +   __intel_set_power_well(dev, true);
>  }
>
> -static void __intel_power_well_put(struct i915_power_well *power_well)
> +static void __intel_power_well_put(struct drm_device *dev,
> +  struct i915_power_well *power_well)
>  {
> WARN_ON(!power_well->count);
> if (!--power_well->count)
> -   __intel_set_power_well(power_well->device, false);
> +   __intel_set_power_well(dev, false);
>  }
>
>  void intel_display_power_get(struct drm_device *dev,
> @@ -5636,7 +5638,7 @@ void intel_display_power_get(struct drm_device *dev,
> power_domains = &dev_priv->power_domains;
>
> mutex_lock(&power_domains->lock);
> -   __intel_power_well_get(&power_domains->power_wells[0]);
> +   __intel_power_well_get(dev, &power_domains->power_wells[0]);
> mutex_unlock(&power_domains->lock);
>  }
>
> @@ -5655,7 +5657,7 @@ void intel_display_power_put(struct drm_device *dev,
> power_domains = &dev_priv->power_domains;
>
> mutex_lock(&power_domains->lock);
> -   __intel_power_well_put(&power_domains->power_wells[0]);
> +   __intel_power_well_put(dev, &power_domains->power_wells[0]);
> mutex_unlock(&power_domains->lock);
>  }
>
> @@ -5664,11 +5666,16 @@ static struct i915_power_domains *hsw_pwr;
>  /* Display audio driver power well request */
>  void i915_request_power_well(void)
>  {
> +   struct drm_i915_private *dev_priv;
> +
> if (WARN_ON(!hsw_pwr))
> return;
>
> +   dev_priv = container_of(hsw_pwr, struct drm_i915_private,
> +   power_domains);
> +
> mutex_lock(&hsw_pwr->lock);
> -   __intel_power_well_get(&hsw_pwr->power_wells[0]);
> +   __intel_power_well_get(dev_priv->dev, &hsw_pwr->power_wells[0]);
> mutex_unlock(&hsw_pwr->lock);
>  }
>  EXPORT_SYMBOL_GPL(i915_request_power_well);
> @@ -5676,11 +5683,16 @@ EXPORT_SYMBOL_GPL(i915_request_power_well);
>  /* Display audio driver power well release */
>  void i915_release_power_well(void)
>  {
> +   struct drm_i915_private *dev_priv;
> +
> if (WARN_ON(!hsw_pwr))
> return;
>
> +   dev_priv = container_of(hsw_pwr, struct drm_i915_private,
> +   power_domains);
> +
> mutex_lock(&hsw_pwr->lock);
> -   __intel_power_well_put(&hsw_pwr->power_wells[0]);
> +   __intel_power_well_put(dev_priv->dev, &hsw_pwr->power_wells[0]);
> mutex_unlock(&hsw_pwr->lock);
>  }
>  EXPORT_SYMBOL_GPL(i915_release_power_well);
> @@ -5695,7 +5707,6 @@ int i915_init_power_well(struct drm_device *dev)
> hsw_pwr = power_domains;
>
> power_well = &power_domains->power_wells[0];
> -   power_well->device = dev;
> power_well->count = 0;
>
> return 0;
> --
> 1.8.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
_

Re: [Intel-gfx] [PATCH v3 4/4] drm/i915: rename i915_init_power_well to i915_init_power_domains

2013-10-25 Thread Paulo Zanoni 
2013/10/25 Imre Deak :
> Similarly rename the other related functions in the power domain
> interface.
>
> Higher level driver code calling these functions knows only about power
> domains, not the underlying power wells which may be different on
> different platforms. Also these functions really init/cleanup/resume
> power domains and only through that all related power wells, so rename
> them accordingly.
>
> Signed-off-by: Imre Deak 

I agree with the "_domains" rename, I think it's an improvement, but
since you're already renaming things, I have to drop my bikeshed: we
have intel_init_power_{well,domains} and
i915_init_power_{well,domains}. IMHO this is really super annoyingly
confusing, because they sound like they do the same thing. I know it's
not your fault, but while you're at it, could you please propose names
to unconfuse this?

i915_init_power_well takes care of initializing the structs and
pointers, while intel_init_power_well is the only one that touches
hardware. A possible suggestion:

- i915_init_power_well becomes intel_init_power_domains (just because
I don't like the "i915_" prefix, since the PM code uses "intel_" for
everything).
- i915_remove_power_well becomes intel_remove_power_domains (to match
the one above)
- intel_init_power_well becomes intel_init_power_domains_hardware or
intel_init_power_wells (since on the HW these things are actually
called "power wells") or intel_init_hw_power_wells (to combine both
suggestions)

Thanks,
Paulo

> ---
>  drivers/gpu/drm/i915/i915_dma.c  |  8 
>  drivers/gpu/drm/i915/i915_drv.c  |  2 +-
>  drivers/gpu/drm/i915/intel_drv.h |  6 +++---
>  drivers/gpu/drm/i915/intel_pm.c  | 10 +-
>  4 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index b722b35..75f02e4 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1311,7 +1311,7 @@ static int i915_load_modeset_init(struct drm_device 
> *dev)
> if (ret)
> goto cleanup_gem_stolen;
>
> -   intel_init_power_well(dev);
> +   intel_init_power_domains(dev);
>
> /* Important: The output setup functions called by modeset_init need
>  * working irqs for e.g. gmbus and dp aux transfers. */
> @@ -1640,7 +1640,7 @@ int i915_driver_load(struct drm_device *dev, unsigned 
> long flags)
> }
>
> if (HAS_POWER_WELL(dev))
> -   i915_init_power_well(dev);
> +   i915_init_power_domains(dev);
>
> if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> ret = i915_load_modeset_init(dev);
> @@ -1668,7 +1668,7 @@ int i915_driver_load(struct drm_device *dev, unsigned 
> long flags)
>
>  out_power_well:
> if (HAS_POWER_WELL(dev))
> -   i915_remove_power_well(dev);
> +   i915_remove_power_domains(dev);
> drm_vblank_cleanup(dev);
>  out_gem_unload:
> if (dev_priv->mm.inactive_shrinker.scan_objects)
> @@ -1711,7 +1711,7 @@ int i915_driver_unload(struct drm_device *dev)
>  * the power well is not enabled, so just enable it in case
>  * we're going to unload/reload. */
> intel_display_set_init_power(dev, true);
> -   i915_remove_power_well(dev);
> +   i915_remove_power_domains(dev);
> }
>
> i915_teardown_sysfs(dev);
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 770c9f8..e4474c6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -597,7 +597,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool 
> restore_gtt_mappings)
> mutex_unlock(&dev->struct_mutex);
> }
>
> -   intel_init_power_well(dev);
> +   intel_init_power_domains(dev);
>
> i915_restore_state(dev);
> intel_opregion_setup(dev);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index bf4394a..ae6d362 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -823,15 +823,15 @@ bool intel_fbc_enabled(struct drm_device *dev);
>  void intel_update_fbc(struct drm_device *dev);
>  void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
>  void intel_gpu_ips_teardown(void);
> -int i915_init_power_well(struct drm_device *dev);
> -void i915_remove_power_well(struct drm_device *dev);
> +int i915_init_power_domains(struct drm_device *dev);
> +void i915_remove_power_domains(struct drm_device *dev);
>  bool intel_display_power_enabled(struct drm_device *dev,
>  enum intel_display_power_domain domain);
>  void intel_display_power_get(struct drm_device *dev,
>  enum intel_display_power_domain domain);
>  void intel_display_power_put(struct drm_device *dev,
>  enum intel_display_power_domain domain);
> -void intel_in

Re: [Intel-gfx] [PATCH 7/7] drm/i915: add i915_get_reset_stats_ioctl

2013-10-25 Thread Ian Romanick 
Since the Mesa merge window is closing soon, I'm finally getting back on
this.  I've pushed a rebase of my old Mesa branch to my fd.o repo

http://cgit.freedesktop.org/~idr/mesa/log/?h=robustness3

I have a couple questions...

1. Has any of this landed an a kernel tree anywhere?

2. Has any support code landed in a libdrm tree anywhere?

3. What method should I use to detect that the kernel has support?  In
early discussions, reset notification was only going to be available on
some GPUs, so there was a getparam to detect actual availability.  I
guess now it's just based on kernel version?

It looks like I should just need to update df87cdd and 61dad8e in my
Mesa tree.

On 07/03/2013 07:22 AM, Mika Kuoppala wrote:
> This ioctl returns reset stats for specified context.
> 
> The struct returned contains context loss counters.
> 
> reset_count:all resets across all contexts
> batch_active:   active batches lost on resets
> batch_pending:  pending batches lost on resets
> 
> v2: get rid of state tracking completely and deliver only counts. Idea
> from Chris Wilson.
> 
> v3: fix commit message
> 
> v4: default context handled inside i915_gem_contest_get_hang_stats
> 
> v5: reset_count only for priviledged process
> 
> v6: ctx=0 needs CAP_SYS_ADMIN for batch_* counters (Chris Wilson)
> 
> v7: context hang stats never returns NULL
> 
> Signed-off-by: Mika Kuoppala 
> Cc: Ian Romanick 
> Cc: Chris Wilson 
> Cc: Daniel Vetter 
> ---
>  drivers/gpu/drm/i915/i915_dma.c |1 +
>  drivers/gpu/drm/i915/i915_drv.c |   34 ++
>  drivers/gpu/drm/i915/i915_drv.h |2 ++
>  include/uapi/drm/i915_drm.h |   17 +
>  4 files changed, 54 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 0e22142..d1a006f 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1889,6 +1889,7 @@ struct drm_ioctl_desc i915_ioctls[] = {
>   DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE, 
> i915_gem_context_create_ioctl, DRM_UNLOCKED),
>   DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, 
> i915_gem_context_destroy_ioctl, DRM_UNLOCKED),
>   DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED),
> + DRM_IOCTL_DEF_DRV(I915_GET_RESET_STATS, i915_get_reset_stats_ioctl, 
> DRM_UNLOCKED),
>  };
>  
>  int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls);
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 33cb973..0d4e3a8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1350,3 +1350,37 @@ int i915_reg_read_ioctl(struct drm_device *dev,
>  
>   return 0;
>  }
> +
> +int i915_get_reset_stats_ioctl(struct drm_device *dev,
> +void *data, struct drm_file *file)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_i915_reset_stats *args = data;
> + struct i915_ctx_hang_stats *hs;
> + int ret;
> +
> + if (args->ctx_id == 0 && !capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + ret = mutex_lock_interruptible(&dev->struct_mutex);
> + if (ret)
> + return ret;
> +
> + hs = i915_gem_context_get_hang_stats(dev, file, args->ctx_id);
> + if (IS_ERR(hs)) {
> + mutex_unlock(&dev->struct_mutex);
> + return PTR_ERR(hs);
> + }
> +
> + if (capable(CAP_SYS_ADMIN))
> + args->reset_count = i915_reset_count(&dev_priv->gpu_error);
> + else
> + args->reset_count = 0;
> +
> + args->batch_active = hs->batch_active;
> + args->batch_pending = hs->batch_pending;
> +
> + mutex_unlock(&dev->struct_mutex);
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1def049..0ca98fa 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2021,6 +2021,8 @@ extern int intel_enable_rc6(const struct drm_device 
> *dev);
>  extern bool i915_semaphore_is_enabled(struct drm_device *dev);
>  int i915_reg_read_ioctl(struct drm_device *dev, void *data,
>   struct drm_file *file);
> +int i915_get_reset_stats_ioctl(struct drm_device *dev, void *data,
> +struct drm_file *file);
>  
>  /* overlay */
>  #ifdef CONFIG_DEBUG_FS
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 923ed7f..29b07fd 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -198,6 +198,7 @@ typedef struct _drm_i915_sarea {
>  #define DRM_I915_GEM_SET_CACHING 0x2f
>  #define DRM_I915_GEM_GET_CACHING 0x30
>  #define DRM_I915_REG_READ0x31
> +#define DRM_I915_GET_RESET_STATS 0x32
>  
>  #define DRM_IOCTL_I915_INIT  DRM_IOW( DRM_COMMAND_BASE + 
> DRM_I915_INIT, drm_i915_init_t)
>  #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + 
> DRM_I915_FLUSH)
> @@ -247,6 +