Re: [Intel-gfx] [PATCH] drm/i915: Dump the contents of the GTT

2013-09-01 Thread Sedat Dilek 
Hi Chris,

is this going into any drm-intel GIT tree?
I found it useful and saw it in your kernel-tree [1].

- Sedat -

[1] 
http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?id=bf91098a1232db771feac66f88720c181ef61967

On Wed, Aug 14, 2013 at 1:35 PM, Chris Wilson ch...@chris-wilson.co.uk wrote:
 Chasing wild speculation that we may be writing the wrong addresses
 into the GTT for stolen objects, I would like to inspect those values.

 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Sedat Dilek sedat.di...@gmail.com
 ---

 Sedak, can you please apply this patch and then attach the contents of
 /sys/kernel/debug/dri/0/i915_gem_gtt with the broken display?

 ---
  drivers/gpu/drm/i915/i915_debugfs.c | 47 
 -
  1 file changed, 46 insertions(+), 1 deletion(-)

 diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
 b/drivers/gpu/drm/i915/i915_debugfs.c
 index 10d864c..4edf65a 100644
 --- a/drivers/gpu/drm/i915/i915_debugfs.c
 +++ b/drivers/gpu/drm/i915/i915_debugfs.c
 @@ -390,6 +390,51 @@ static int i915_gem_object_info(struct seq_file *m, 
 void* data)
 return 0;
  }

 +static int i915_gem_gtt_contents(struct seq_file *m, struct drm_device *dev)
 +{
 +   struct drm_i915_private *dev_priv = dev-dev_private;
 +   gen6_gtt_pte_t __iomem *gtt_entries;
 +   gen6_gtt_pte_t scratch_pte;
 +   gen6_gtt_pte_t zero[8] = {};
 +   int i, j, last_zero = 0;
 +   int ret;
 +
 +   if (INTEL_INFO(dev)-gen  6)
 +   return 0;
 +
 +   ret = mutex_lock_interruptible(dev-struct_mutex);
 +   if (ret)
 +   return ret;
 +
 +   gtt_entries = (gen6_gtt_pte_t __iomem *)dev_priv-gtt.gsm;
 +   scratch_pte = 
 dev_priv-gtt.base.pte_encode(dev_priv-gtt.base.scratch.addr, 
 I915_CACHE_LLC);
 +   for (i = 0; i  gtt_total_entries(dev_priv-gtt); i += 8) {
 +   gen6_gtt_pte_t pte[8];
 +   int this_zero;
 +
 +   for (j = 0; j  8; j++) {
 +   pte[j] = ioread32(gtt_entries[i+j]);
 +   if (pte[j] == scratch_pte)
 +   pte[j] = 0;
 +   }
 +
 +   this_zero = memcmp(pte, zero, sizeof(pte)) == 0;
 +   if (last_zero  this_zero) {
 +   if (last_zero++ == 1)
 +   seq_puts(m, ...\n);
 +   continue;
 +   }
 +
 +   seq_printf(m, [%08x] %08x %08x %08x %08x %08x %08x %08x 
 %08x\n,
 +  i, pte[0], pte[1], pte[2], pte[3], pte[4], pte[5], 
 pte[6], pte[7]);
 +   last_zero = this_zero;
 +   }
 +
 +   mutex_unlock(dev-struct_mutex);
 +
 +   return 0;
 +}
 +
  static int i915_gem_gtt_info(struct seq_file *m, void *data)
  {
 struct drm_info_node *node = (struct drm_info_node *) m-private;
 @@ -422,7 +467,7 @@ static int i915_gem_gtt_info(struct seq_file *m, void 
 *data)
 seq_printf(m, Total %d objects, %zu bytes, %zu GTT size\n,
count, total_obj_size, total_gtt_size);

 -   return 0;
 +   return i915_gem_gtt_contents(m, dev);
  }

  static int i915_gem_pageflip_info(struct seq_file *m, void *data)
 --
 1.8.4.rc2

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

Re: [Intel-gfx] drm/i915: split PCI IDs out into i915_drm.h v4

2013-09-01 Thread Sedat Dilek 
Hi,

this is really cool!

Looking at this from [1]:

[ include/drm/i915_pciids.h ]
...
+#define INTEL_SNB_M_IDS(info) \
+ INTEL_VGA_DEVICE(0x0106, info), \
+ INTEL_VGA_DEVICE(0x0116, info), \ --- I have this one! GT2 mobile?
+ INTEL_VGA_DEVICE(0x0126, info)
+
+#define INTEL_IVB_M_IDS(info) \
+ INTEL_VGA_DEVICE(0x0156, info), /* GT1 mobile */ \
+ INTEL_VGA_DEVICE(0x0166, info) /* GT2 mobile */

I remember to have seen GT2 for my Sandybridge system (Samsung
series-5 ultrabook) in the logs.
Other defines have comments like this.

[ lspci -vmnn ]

Device: 00:02.0
Class:  VGA compatible controller [0300]
Vendor: Intel Corporation [8086]
Device: 2nd Generation Core Processor Family Integrated Graphics
Controller [0116]
SVendor:Samsung Electronics Co Ltd [144d]
SDevice:Device [c0c7]
Rev:09

Was the comments forgotten for SNB-mobile GPUs?

Regards,
- Sedat -

[1] 
http://cgit.freedesktop.org/~danvet/drm-intel/commit/?h=for-linux-nextid=09e60dcc1f6a2f3bbf6b97e957061e97f8cd297c
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] drm/i915: split PCI IDs out into i915_drm.h v4

2013-09-01 Thread Sedat Dilek 
On Sun, Sep 1, 2013 at 4:10 PM, Sedat Dilek sedat.di...@gmail.com wrote:

[...]

 [ include/drm/i915_pciids.h ]
 ...
 +#define INTEL_SNB_M_IDS(info) \
 + INTEL_VGA_DEVICE(0x0106, info), \
 + INTEL_VGA_DEVICE(0x0116, info), \ --- I have this one! GT2 mobile?
 + INTEL_VGA_DEVICE(0x0126, info)
 +
 +#define INTEL_IVB_M_IDS(info) \
 + INTEL_VGA_DEVICE(0x0156, info), /* GT1 mobile */ \
 + INTEL_VGA_DEVICE(0x0166, info) /* GT2 mobile */

 I remember to have seen GT2 for my Sandybridge system (Samsung
 series-5 ultrabook) in the logs.

$ grep -i sandy /var/log/Xorg.0.log
[18.160] (II) intel(0): SNA initialized with Sandybridge (gen6, gt2) backend

BTW, is there somewhere on the Wild Wild Internet a doc/wiki where I
can have a human-readable list of Intel GPU hardware (there exist
GenX and GTY)?
The X RadeonFeature wiki has a section Decoder ring for engineering
vs marketing names in [1].

As a last thing, I noticed that brand-names like SandyBridge are
written differently in the Linux graphics stack (kernel-drm, libdrm,
mesa3d and intel-ddx). I can't say what is the official brand-name.
( The reason why I ask is for example searching for patterns in the sources. )

$ dmesg | grep -i sandy
[0.081443] Performance Events: PEBS fmt1+, 16-deep LBR,
SandyBridge events, full-width counters, Intel PMU driver.

$ grep -i sandy /var/log/Xorg.0.log
[18.160] (II) intel(0): SNA initialized with Sandybridge (gen6, gt2) backend

Thanks.

- Sedat -

[1] http://wiki.x.org/wiki/RadeonFeature/#index5h2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] drm/i915: split PCI IDs out into i915_drm.h v4

2013-09-01 Thread Chris Wilson 
On Sun, Sep 01, 2013 at 04:28:39PM +0200, Sedat Dilek wrote:
 On Sun, Sep 1, 2013 at 4:10 PM, Sedat Dilek sedat.di...@gmail.com wrote:
 
 [...]
 
  [ include/drm/i915_pciids.h ]
  ...
  +#define INTEL_SNB_M_IDS(info) \
  + INTEL_VGA_DEVICE(0x0106, info), \
  + INTEL_VGA_DEVICE(0x0116, info), \ --- I have this one! GT2 mobile?
  + INTEL_VGA_DEVICE(0x0126, info)
  +
  +#define INTEL_IVB_M_IDS(info) \
  + INTEL_VGA_DEVICE(0x0156, info), /* GT1 mobile */ \
  + INTEL_VGA_DEVICE(0x0166, info) /* GT2 mobile */
 
  I remember to have seen GT2 for my Sandybridge system (Samsung
  series-5 ultrabook) in the logs.
 
 $ grep -i sandy /var/log/Xorg.0.log
 [18.160] (II) intel(0): SNA initialized with Sandybridge (gen6, gt2) 
 backend
 
 BTW, is there somewhere on the Wild Wild Internet a doc/wiki where I
 can have a human-readable list of Intel GPU hardware (there exist
 GenX and GTY)?
 The X RadeonFeature wiki has a section Decoder ring for engineering
 vs marketing names in [1].
 
 As a last thing, I noticed that brand-names like SandyBridge are
 written differently in the Linux graphics stack (kernel-drm, libdrm,
 mesa3d and intel-ddx). I can't say what is the official brand-name.
 ( The reason why I ask is for example searching for patterns in the sources. )
 
 $ dmesg | grep -i sandy
 [0.081443] Performance Events: PEBS fmt1+, 16-deep LBR,
 SandyBridge events, full-width counters, Intel PMU driver.
 
 $ grep -i sandy /var/log/Xorg.0.log
 [18.160] (II) intel(0): SNA initialized with Sandybridge (gen6, gt2) 
 backend

We have to be careful as Sandybridge isn't a brand or product name but a
code name, and Intel marketing gets upset if we put the codenames into
user visible strings. (I can understand their need for control over
product image and branding, but the codenames are much easier to
understand!) The popular form for the *bridge, *well, *trail, *view is
as one word with a single leading capital letter. The codenames are,
I believe, or at least once were, the names of geographic features
around the Intel campuses. And different types of features (rivers,
hills, etc) were used to denote different types/combinations of chips.

Wikipedia is the best source for such information as product to code
name to features. Though ark.intel.com has all the information if you
have a glossary (wikipedia) to hand, and have product names to search.
-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 1/2] drm/i915: Modify DP set clock to accomodate more eDP timings.

2013-09-01 Thread Lee, Chon Ming 
On 08/30 10:28, Jani Nikula wrote:
 On Fri, 30 Aug 2013, Chon Ming Lee chon.ming@intel.com wrote:
  eDP 1.4 supports 4-5 extra link rates in additional to current 2 link
  rate.  Create a structure to store the DPLL divisor data to improve
  readability.
 
 DP 1.2 already supports 3 link rates, right?

Yes, 3 link rate for DP 1.2, but most of the older intel gfx doesn't support the
highest 5.4Gbps link rate yet.

 
  Signed-off-by: Chon Ming Lee chon.ming@intel.com
  ---
   drivers/gpu/drm/i915/intel_dp.c |   48 
  +++---
   1 files changed, 24 insertions(+), 24 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/intel_dp.c 
  b/drivers/gpu/drm/i915/intel_dp.c
  index 2151d13..ab8a5ff 100644
  --- a/drivers/gpu/drm/i915/intel_dp.c
  +++ b/drivers/gpu/drm/i915/intel_dp.c
  @@ -38,6 +38,19 @@
   
   #define DP_LINK_CHECK_TIMEOUT  (10 * 1000)
   
  +struct dp_link_dpll{
 
 Missing space before {.
 
  +   int link_bw;
  +   struct dpll dpll;
  +};
  +
  +static const struct dp_link_dpll gen4_dpll[] = 
  +   {{ DP_LINK_BW_1_62, {2,23,8,2,10,0,0,0,0}},
  +   { DP_LINK_BW_2_7,  {1,14,2,1,10,0,0,0,0}}};
  +
  +static const struct dp_link_dpll pch_dpll[] = 
  +   {{ DP_LINK_BW_1_62, {1,12,9,2,10,0,0,0,0}},
  +   { DP_LINK_BW_2_7,  {2,14,8,1,10,0,0,0,0}}};
  +
 
 Please switch these to use C99 designated initializers for clarity,
 something like this:
 
 static const struct dp_link_dpll gen4_dpll[] = {
   {
   .link_bw = DP_LINK_BW_1_62,
   .dpll = {
   .n = 2,
   .m1 = 23, m2 = 8,
   p1 = 2, p2 = 10, 
   },
   }, {
   .link_bw = DP_LINK_BW_2_7,
   .dpll = {
   .n = 1,
   .m1 = 14, m2 = 2,
   p1 = 1, p2 = 10, 
   },
   }
 };
 
Thanks, will make the correction.

   /**
* is_edp - is the given port attached to an eDP panel (either CPU or PCH)
* @intel_dp: DP struct
  @@ -649,37 +662,24 @@ intel_dp_set_clock(struct intel_encoder *encoder,
 struct intel_crtc_config *pipe_config, int link_bw)
   {
  struct drm_device *dev = encoder-base.dev;
  +   int i;
   
  if (IS_G4X(dev)) {
  -   if (link_bw == DP_LINK_BW_1_62) {
  -   pipe_config-dpll.p1 = 2;
  -   pipe_config-dpll.p2 = 10;
  -   pipe_config-dpll.n = 2;
  -   pipe_config-dpll.m1 = 23;
  -   pipe_config-dpll.m2 = 8;
  -   } else {
  -   pipe_config-dpll.p1 = 1;
  -   pipe_config-dpll.p2 = 10;
  -   pipe_config-dpll.n = 1;
  -   pipe_config-dpll.m1 = 14;
  -   pipe_config-dpll.m2 = 2;
  +   for(i = 0; i  sizeof(gen4_dpll) / sizeof(struct dp_link_dpll); 
  i++) {
 
 Please use i  ARRAY_SIZE(gen4_dpll).

Already make this change.  After sent out the patch, realize I should this
ARRAY_SIZE.  Thanks to point this out.  
 
  +   if (link_bw == gen4_dpll[i].link_bw){
  +   pipe_config-dpll = gen4_dpll[i].dpll;
  +   break;
  +   }
  }
 
 The old if-else used some values even if link_bw was bogus. I'm not sure
 how likely that is. But if the link_bw is not found in the table for
 some obscure reason (e.g. the hw doesn't support the link rate), this
 fails. Perhaps we should have a test for i == ARRAY_SIZE(gen4_dpll) here
 and complain loudly if we hit that, and perhaps use a fallback value.
 

In intel_dp_compute_config, it will only assign either one of two link rates 
into link_bw before calling this function.  The link_bw won't be out of range.  

I would think the checking should be done prior to calling this function.  For
example, in eDP 1.4, instead of supporting more link rates, there is possible to
use single lane, but choose the largest link rate, or use 4 lanes, with lower
link rate.  If fail out here, might be too late to recover.   

  pipe_config-clock_set = true;
  } else if (IS_HASWELL(dev)) {
  /* Haswell has special-purpose DP DDI clocks. */
  } else if (HAS_PCH_SPLIT(dev)) {
  -   if (link_bw == DP_LINK_BW_1_62) {
  -   pipe_config-dpll.n = 1;
  -   pipe_config-dpll.p1 = 2;
  -   pipe_config-dpll.p2 = 10;
  -   pipe_config-dpll.m1 = 12;
  -   pipe_config-dpll.m2 = 9;
  -   } else {
  -   pipe_config-dpll.n = 2;
  -   pipe_config-dpll.p1 = 1;
  -   pipe_config-dpll.p2 = 10;
  -   pipe_config-dpll.m1 = 14;
  -   pipe_config-dpll.m2 = 8;
  +   for(i = 0; i  sizeof(pch_dpll) / sizeof(struct dp_link_dpll);

Re: [Intel-gfx] drm/i915: split PCI IDs out into i915_drm.h v4

2013-09-01 Thread Sedat Dilek 
On Sun, Sep 1, 2013 at 5:05 PM, Chris Wilson ch...@chris-wilson.co.uk wrote:
 On Sun, Sep 01, 2013 at 04:28:39PM +0200, Sedat Dilek wrote:
 On Sun, Sep 1, 2013 at 4:10 PM, Sedat Dilek sedat.di...@gmail.com wrote:

 [...]

  [ include/drm/i915_pciids.h ]
  ...
  +#define INTEL_SNB_M_IDS(info) \
  + INTEL_VGA_DEVICE(0x0106, info), \
  + INTEL_VGA_DEVICE(0x0116, info), \ --- I have this one! GT2 mobile?
  + INTEL_VGA_DEVICE(0x0126, info)
  +
  +#define INTEL_IVB_M_IDS(info) \
  + INTEL_VGA_DEVICE(0x0156, info), /* GT1 mobile */ \
  + INTEL_VGA_DEVICE(0x0166, info) /* GT2 mobile */
 
  I remember to have seen GT2 for my Sandybridge system (Samsung
  series-5 ultrabook) in the logs.

 $ grep -i sandy /var/log/Xorg.0.log
 [18.160] (II) intel(0): SNA initialized with Sandybridge (gen6, gt2) 
 backend

 BTW, is there somewhere on the Wild Wild Internet a doc/wiki where I
 can have a human-readable list of Intel GPU hardware (there exist
 GenX and GTY)?
 The X RadeonFeature wiki has a section Decoder ring for engineering
 vs marketing names in [1].

 As a last thing, I noticed that brand-names like SandyBridge are
 written differently in the Linux graphics stack (kernel-drm, libdrm,
 mesa3d and intel-ddx). I can't say what is the official brand-name.
 ( The reason why I ask is for example searching for patterns in the sources. 
 )

 $ dmesg | grep -i sandy
 [0.081443] Performance Events: PEBS fmt1+, 16-deep LBR,
 SandyBridge events, full-width counters, Intel PMU driver.

 $ grep -i sandy /var/log/Xorg.0.log
 [18.160] (II) intel(0): SNA initialized with Sandybridge (gen6, gt2) 
 backend

 We have to be careful as Sandybridge isn't a brand or product name but a
 code name, and Intel marketing gets upset if we put the codenames into
 user visible strings. (I can understand their need for control over
 product image and branding, but the codenames are much easier to
 understand!) The popular form for the *bridge, *well, *trail, *view is
 as one word with a single leading capital letter. The codenames are,
 I believe, or at least once were, the names of geographic features
 around the Intel campuses. And different types of features (rivers,
 hills, etc) were used to denote different types/combinations of chips.

 Wikipedia is the best source for such information as product to code
 name to features. Though ark.intel.com has all the information if you
 have a glossary (wikipedia) to hand, and have product names to search.

No success with searching 1st for sandybridge (one word) :-(, so
official docs have a space (Sandy Bridge)

[1] says:
Sandy Bridge is the codename for both the Intel microarchitecture
innovation following Nehalem and generally for the associated family
of 32nm processors based upon that microarchitecture.

For the GPU there is Processor Graphics.
So mine seems to be called Intel® HD Graphics 3000.
Hmm, I remember darkly intel-ddx uses the latter.
Anyway, somehow all that stuff is still confusing me.

- Sedat -

[1] http://ark.intel.com/products/codename/29900/Sandy-Bridge?q=Sandy
___
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: Move Valleyview DP DPLL divisor calc to intel_dp_set_clock

2013-09-01 Thread Lee, Chon Ming 
On 08/30 11:00, Jani Nikula wrote:
 
 [Okay, I missed Daniel's review, and noticed I hadn't actually hit send
 on this one either... but here goes anyway...]
 
 On Fri, 30 Aug 2013, Chon Ming Lee chon.ming@intel.com wrote:
  For DP pll settings, there is only two golden configs.  Instead of running
  through the algorithm to determine it, hardcode the value and get it
  determine in intel_dp_set_clock.
 
  Signed-off-by: Chon Ming Lee chon.ming@intel.com
  ---
   drivers/gpu/drm/i915/intel_display.c |   22 --
   drivers/gpu/drm/i915/intel_dp.c  |   12 +++-
   2 files changed, 15 insertions(+), 19 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/intel_display.c 
  b/drivers/gpu/drm/i915/intel_display.c
  index f526ea9..453fa16 100644
  --- a/drivers/gpu/drm/i915/intel_display.c
  +++ b/drivers/gpu/drm/i915/intel_display.c
  @@ -339,19 +339,6 @@ static const intel_limit_t intel_limits_vlv_hdmi = {
  .p2_slow = 2, .p2_fast = 20 },
   };
   
  -static const intel_limit_t intel_limits_vlv_dp = {
  -   .dot = { .min = 25000, .max = 27 },
  -   .vco = { .min = 400, .max = 600 },
  -   .n = { .min = 1, .max = 7 },
  -   .m = { .min = 22, .max = 450 },
  -   .m1 = { .min = 2, .max = 3 },
  -   .m2 = { .min = 11, .max = 156 },
  -   .p = { .min = 10, .max = 30 },
  -   .p1 = { .min = 1, .max = 3 },
  -   .p2 = { .dot_limit = 27,
  -   .p2_slow = 2, .p2_fast = 20 },
  -};
  -
   static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc,
  int refclk)
   {
  @@ -414,10 +401,8 @@ static const intel_limit_t *intel_limit(struct 
  drm_crtc *crtc, int refclk)
  } else if (IS_VALLEYVIEW(dev)) {
  if (intel_pipe_has_type(crtc, INTEL_OUTPUT_ANALOG))
  limit = intel_limits_vlv_dac;
  -   else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
  +   else 
  limit = intel_limits_vlv_hdmi;
  -   else
  -   limit = intel_limits_vlv_dp;
  } else if (!IS_GEN2(dev)) {
  if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
  limit = intel_limits_i9xx_lvds;
  @@ -4889,15 +4874,16 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
  }
   
  refclk = i9xx_get_refclk(crtc, num_connectors);
  +   
  +   limit = intel_limit(crtc, refclk);
 
 Did you move this here just to avoid the warning about uninitialized
 limit? It's a bit ugly... but then again the the whole is_dsi vs. not is
 rather ugly already. *shrug*.
 
Yes, correct.  I can reverse it but, just have to add another limit =
intel_limit(crtc, refclk); in this if statement.

if (!is_dsi  is_lvds  dev_priv-lvds_downclock_avail) {

Will it more make sense?

   
  -   if (!is_dsi) {
  +   if (!is_dsi  !intel_crtc-config.clock_set) {
  /*
   * Returns a set of divisors for the desired target clock with
   * the given refclk, or FALSE.  The returned values represent
   * the clock equation: reflck * (5 * (m1 + 2) + (m2 + 2)) / (n +
   * 2) / p1 / p2.
   */
  -   limit = intel_limit(crtc, refclk);
  ok = dev_priv-display.find_dpll(limit, crtc,
   intel_crtc-config.port_clock,
   refclk, NULL, clock);
  diff --git a/drivers/gpu/drm/i915/intel_dp.c 
  b/drivers/gpu/drm/i915/intel_dp.c
  index ab8a5ff..89a2606 100644
  --- a/drivers/gpu/drm/i915/intel_dp.c
  +++ b/drivers/gpu/drm/i915/intel_dp.c
  @@ -51,6 +51,10 @@ static const struct dp_link_dpll pch_dpll[] =
  {{ DP_LINK_BW_1_62, {1,12,9,2,10,0,0,0,0}},
  { DP_LINK_BW_2_7,  {2,14,8,1,10,0,0,0,0}}};
   
  +static const struct dp_link_dpll vlv_dpll[] =
  +   {{ DP_LINK_BW_1_62, {5,3,81,3,2,0,0,0,0}},
  +   { DP_LINK_BW_2_7, {1,2,27,2,2,0,0,0,0}}};
  +
   /**
* is_edp - is the given port attached to an eDP panel (either CPU or PCH)
* @intel_dp: DP struct
  @@ -683,7 +687,13 @@ intel_dp_set_clock(struct intel_encoder *encoder,
  }
  pipe_config-clock_set = true;
  } else if (IS_VALLEYVIEW(dev)) {
  -   /* FIXME: Need to figure out optimized DP clocks for vlv. */
  +   for(i = 0; i  sizeof(vlv_dpll) / sizeof(struct dp_link_dpll); 
  i++) {
  +   if (link_bw == vlv_dpll[i].link_bw){ 
  +   pipe_config-dpll = vlv_dpll[i].dpll;
  +   break;
  +   }
  +   }
  +   pipe_config-clock_set = true;
 
 You now have three similar loops in the function. A follow-up patch
 could pick the table to use in the if branches, and have a single loop
 at the end. You could handle the array size by having .link_bw = 0 in
 the last entry as a stop condition, and

[Intel-gfx] [PATCH] drm/i915: Avoid flicker with horizontal panning on 830GM

2013-09-01 Thread Thomas Richter 

Dear intel-gfx developers,

When panning is enabled on the 830GM, horizontal panning creates a lot 
of flickering on specific pixel positions.
After testing, I found that the reason for this is that panning works by 
altering the frame origin pointer, which,
however, has certain alignment restrictions. If the pointer is not 
aligned correctly, the screen starts to flicker

as, probably, DMA fails.

The following patch against drm/i915/intel_display.c fixes the issue by 
ensuring correct alignment. As result,
horizontal panning works correctly, but is a bit jumpy. Unclear 
whether the problem affects any other

chipset revisions, thus the patch is currently only enabled for rev.2.

Greetings,
Thomas


diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c

index bcb62fe..8304e30 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1987,8 +1987,13 @@ static int i9xx_update_plane(struct drm_crtc 
*crtc, struct drm_framebuffer *fb,


I915_WRITE(reg, dspcntr);

-   linear_offset = y * fb-pitches[0] + x * (fb-bits_per_pixel / 8);
-
+   if (INTEL_INFO(dev)-gen  2) {
+ linear_offset = y * fb-pitches[0] + x * (fb-bits_per_pixel / 8);
+   } else {
+ /* align the linear offset to 64 pixel boundaries */
+ linear_offset = y * fb-pitches[0] + (x  -32) * 
(fb-bits_per_pixel / 8);

+   }
+
if (INTEL_INFO(dev)-gen = 4) {
intel_crtc-dspaddr_offset =
intel_gen4_compute_page_offset(x, y, 
obj-tiling_mode,


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

Re: [Intel-gfx] [PATCH] drm/i915: Dump the contents of the GTT

2013-09-01 Thread Ben Widawsky 
On Sun, Sep 01, 2013 at 03:59:00PM +0200, Sedat Dilek wrote:
 Hi Chris,
 
 is this going into any drm-intel GIT tree?
 I found it useful and saw it in your kernel-tree [1].
 
 - Sedat -
 
 [1] 
 http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?id=bf91098a1232db771feac66f88720c181ef61967

It can already be accomplished with the intel_gtt tool in the
intel-gpu-tools suite.

 
 On Wed, Aug 14, 2013 at 1:35 PM, Chris Wilson ch...@chris-wilson.co.uk 
 wrote:
  Chasing wild speculation that we may be writing the wrong addresses
  into the GTT for stolen objects, I would like to inspect those values.
 
  Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
  Cc: Sedat Dilek sedat.di...@gmail.com
  ---
 
  Sedak, can you please apply this patch and then attach the contents of
  /sys/kernel/debug/dri/0/i915_gem_gtt with the broken display?
 
  ---
   drivers/gpu/drm/i915/i915_debugfs.c | 47 
  -
   1 file changed, 46 insertions(+), 1 deletion(-)
 
  diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
  b/drivers/gpu/drm/i915/i915_debugfs.c
  index 10d864c..4edf65a 100644
  --- a/drivers/gpu/drm/i915/i915_debugfs.c
  +++ b/drivers/gpu/drm/i915/i915_debugfs.c
  @@ -390,6 +390,51 @@ static int i915_gem_object_info(struct seq_file *m, 
  void* data)
  return 0;
   }
 
  +static int i915_gem_gtt_contents(struct seq_file *m, struct drm_device 
  *dev)
  +{
  +   struct drm_i915_private *dev_priv = dev-dev_private;
  +   gen6_gtt_pte_t __iomem *gtt_entries;
  +   gen6_gtt_pte_t scratch_pte;
  +   gen6_gtt_pte_t zero[8] = {};
  +   int i, j, last_zero = 0;
  +   int ret;
  +
  +   if (INTEL_INFO(dev)-gen  6)
  +   return 0;
  +
  +   ret = mutex_lock_interruptible(dev-struct_mutex);
  +   if (ret)
  +   return ret;
  +
  +   gtt_entries = (gen6_gtt_pte_t __iomem *)dev_priv-gtt.gsm;
  +   scratch_pte = 
  dev_priv-gtt.base.pte_encode(dev_priv-gtt.base.scratch.addr, 
  I915_CACHE_LLC);
  +   for (i = 0; i  gtt_total_entries(dev_priv-gtt); i += 8) {
  +   gen6_gtt_pte_t pte[8];
  +   int this_zero;
  +
  +   for (j = 0; j  8; j++) {
  +   pte[j] = ioread32(gtt_entries[i+j]);
  +   if (pte[j] == scratch_pte)
  +   pte[j] = 0;
  +   }
  +
  +   this_zero = memcmp(pte, zero, sizeof(pte)) == 0;
  +   if (last_zero  this_zero) {
  +   if (last_zero++ == 1)
  +   seq_puts(m, ...\n);
  +   continue;
  +   }
  +
  +   seq_printf(m, [%08x] %08x %08x %08x %08x %08x %08x %08x 
  %08x\n,
  +  i, pte[0], pte[1], pte[2], pte[3], pte[4], 
  pte[5], pte[6], pte[7]);
  +   last_zero = this_zero;
  +   }
  +
  +   mutex_unlock(dev-struct_mutex);
  +
  +   return 0;
  +}
  +
   static int i915_gem_gtt_info(struct seq_file *m, void *data)
   {
  struct drm_info_node *node = (struct drm_info_node *) m-private;
  @@ -422,7 +467,7 @@ static int i915_gem_gtt_info(struct seq_file *m, void 
  *data)
  seq_printf(m, Total %d objects, %zu bytes, %zu GTT size\n,
 count, total_obj_size, total_gtt_size);
 
  -   return 0;
  +   return i915_gem_gtt_contents(m, dev);
   }
 
   static int i915_gem_pageflip_info(struct seq_file *m, void *data)
  --
  1.8.4.rc2
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Intel-gfx] [PATCH 1/7] intel_reg_dumper: Silence GCC for uninitialized clock

2013-09-01 Thread Ben Widawsky 
GCC 4.8.1 seems to think clock may be uninitialized.

Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 tools/intel_reg_dumper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/intel_reg_dumper.c b/tools/intel_reg_dumper.c
index bf3452c..1f4e877 100644
--- a/tools/intel_reg_dumper.c
+++ b/tools/intel_reg_dumper.c
@@ -1573,7 +1573,7 @@ DEBUGSTRING(ilk_debug_pp_control)
 
 DEBUGSTRING(hsw_debug_port_clk_sel)
 {
-   const char *clock;
+   const char *clock = NULL;
 
switch ((val  29 )  7) {
case 0:
-- 
1.8.4

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

[Intel-gfx] [PATCH 2/7] gem_vmap_blits: Demote warning to note

2013-09-01 Thread Ben Widawsky 
The warning that vmap isn't supported is useful, but it shouldn't get in
the way of developers (or distros) being able to use -Werror.

Cc: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 tests/gem_vmap_blits.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/gem_vmap_blits.c b/tests/gem_vmap_blits.c
index c09bcfc..3e8e458 100644
--- a/tests/gem_vmap_blits.c
+++ b/tests/gem_vmap_blits.c
@@ -51,7 +51,7 @@
 #include intel_gpu_tools.h
 
 #if !defined(I915_PARAM_HAS_VMAP)
-#warning No vmap support in drm, skipping
+#pragma message(No vmap support in drm, skipping)
 int main(int argc, char **argv)
 {
fprintf(stderr, No vmap support in drm.\n);
-- 
1.8.4

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

[Intel-gfx] [PATCH 3/7] tools/Makefile.am: use -Werror

2013-09-01 Thread Ben Widawsky 
Our tools should always compile without warnings. We use them to get
debug output from end users, and ignoring warnings could be detrimental.
Tests are a different matter.

Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 tools/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/Makefile.am b/tools/Makefile.am
index 47bd5b3..13cc03a 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -38,7 +38,7 @@ noinst_PROGRAMS = \
 dist_bin_SCRIPTS = intel_gpu_abrt
 
 AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib
-AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS)
+AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) 
-Werror
 LDADD = $(top_builddir)/lib/libintel_tools.la $(DRM_LIBS) $(PCIACCESS_LIBS) 
$(CAIRO_LIBS)
 
 intel_dump_decode_SOURCES =\
-- 
1.8.4

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

[Intel-gfx] [PATCH 4/7] intel_gtt: Use function to get the physical address

2013-09-01 Thread Ben Widawsky 
The GTT PTEs that the tool is trying to compare is really about
addresses, and not the PTE itself. To accomplish this, make which
calculates the physical address we actually want.

This commit itself doesn't change any functionality; just the wording in
the code.

Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 tools/intel_gtt.c | 26 +++---
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/tools/intel_gtt.c b/tools/intel_gtt.c
index 7885610..32a6618 100644
--- a/tools/intel_gtt.c
+++ b/tools/intel_gtt.c
@@ -34,16 +34,20 @@
 
 #include intel_gpu_tools.h
 
-#define INGTT(offset) (*(volatile uint32_t *)(gtt + (offset) / (KB(4) / 4)))
-
 #define KB(x) ((x) * 1024)
 #define MB(x) ((x) * 1024 * 1024)
+unsigned char *gtt;
+
+#define INGTT(offset) (*(volatile uint32_t *)(gtt + (offset) / (KB(4) / 4)))
+static uint64_t get_phys(uint32_t pt_offset)
+{
+   return INGTT(pt_offset);
+}
 
 int main(int argc, char **argv)
 {
struct pci_device *pci_dev;
int start, aper_size;
-   unsigned char *gtt;
uint32_t devid;
int flag[] = {
PCI_DEV_MAP_FLAG_WRITE_COMBINE,
@@ -90,15 +94,15 @@ int main(int argc, char **argv)
aper_size = pci_dev-regions[2].size;
 
for (start = 0; start  aper_size; start += KB(4)) {
-   uint32_t start_pte = INGTT(start);
+   uint32_t start_phys = INGTT(start);
uint32_t end;
int constant_length = 0;
int linear_length = 0;
 
/* Check if it's a linear sequence */
for (end = start + KB(4); end  aper_size; end += KB(4)) {
-   uint32_t end_pte = INGTT(end);
-   if (end_pte == start_pte + (end - start))
+   uint32_t end_phys = INGTT(end);
+   if (end_phys == start_phys + (end - start))
linear_length++;
else
break;
@@ -107,27 +111,27 @@ int main(int argc, char **argv)
printf(0x%08x - 0x%08x: linear from 
   0x%08x to 0x%08x\n,
   start, end - KB(4),
-  start_pte, start_pte + (end - start) - KB(4));
+  start_phys, start_phys + (end - start) - KB(4));
start = end - KB(4);
continue;
}
 
/* Check if it's a constant sequence */
for (end = start + KB(4); end  aper_size; end += KB(4)) {
-   uint32_t end_pte = INGTT(end);
-   if (end_pte == start_pte)
+   uint32_t end_phys = INGTT(end);
+   if (end_phys == start_phys)
constant_length++;
else
break;
}
if (constant_length  0) {
printf(0x%08x - 0x%08x: constant 0x%08x\n,
-  start, end - KB(4), start_pte);
+  start, end - KB(4), start_phys);
start = end - KB(4);
continue;
}
 
-   printf(0x%08x: 0x%08x\n, start, start_pte);
+   printf(0x%08x: 0x%08x\n, start, start_phys);
}
 
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 5/7] intel_gtt: Properly support gen6+ GTT PTEs

2013-09-01 Thread Ben Widawsky 
This finishes the objective in the last patch which was to actually deal
with physical addresses, and not the PTEs.

GEN6+ Provided support for physical addresses above 4GB. I'm not
actually sure what Ironlake supported, and don't feel like firing up the
timemachine.

Haswell caveat is coming up next.

Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 tools/intel_gtt.c | 26 ++
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/tools/intel_gtt.c b/tools/intel_gtt.c
index 32a6618..874a4f6 100644
--- a/tools/intel_gtt.c
+++ b/tools/intel_gtt.c
@@ -25,6 +25,8 @@
  *
  */
 
+#define __STDC_FORMAT_MACROS
+#include inttypes.h
 #include stdio.h
 #include stdlib.h
 #include string.h
@@ -37,18 +39,26 @@
 #define KB(x) ((x) * 1024)
 #define MB(x) ((x) * 1024 * 1024)
 unsigned char *gtt;
+uint32_t devid;
 
 #define INGTT(offset) (*(volatile uint32_t *)(gtt + (offset) / (KB(4) / 4)))
 static uint64_t get_phys(uint32_t pt_offset)
 {
-   return INGTT(pt_offset);
+   uint64_t pae = 0;
+   uint64_t phys = INGTT(pt_offset);
+
+   if (intel_gen(devid)  6)
+   return phys;
+
+   pae = (phys  0xff0)  28;
+
+   return (phys | pae)  ~0xfff;
 }
 
 int main(int argc, char **argv)
 {
struct pci_device *pci_dev;
int start, aper_size;
-   uint32_t devid;
int flag[] = {
PCI_DEV_MAP_FLAG_WRITE_COMBINE,
PCI_DEV_MAP_FLAG_WRITABLE,
@@ -94,14 +104,14 @@ int main(int argc, char **argv)
aper_size = pci_dev-regions[2].size;
 
for (start = 0; start  aper_size; start += KB(4)) {
-   uint32_t start_phys = INGTT(start);
+   uint64_t start_phys = get_phys(start);
uint32_t end;
int constant_length = 0;
int linear_length = 0;
 
/* Check if it's a linear sequence */
for (end = start + KB(4); end  aper_size; end += KB(4)) {
-   uint32_t end_phys = INGTT(end);
+   uint64_t end_phys = get_phys(end);
if (end_phys == start_phys + (end - start))
linear_length++;
else
@@ -109,7 +119,7 @@ int main(int argc, char **argv)
}
if (linear_length  0) {
printf(0x%08x - 0x%08x: linear from 
-  0x%08x to 0x%08x\n,
+  0x% PRIx64  to 0x% PRIx64 \n,
   start, end - KB(4),
   start_phys, start_phys + (end - start) - KB(4));
start = end - KB(4);
@@ -118,20 +128,20 @@ int main(int argc, char **argv)
 
/* Check if it's a constant sequence */
for (end = start + KB(4); end  aper_size; end += KB(4)) {
-   uint32_t end_phys = INGTT(end);
+   uint64_t end_phys = get_phys(end);
if (end_phys == start_phys)
constant_length++;
else
break;
}
if (constant_length  0) {
-   printf(0x%08x - 0x%08x: constant 0x%08x\n,
+   printf(0x%08x - 0x%08x: constant 0x% PRIx64 \n,
   start, end - KB(4), start_phys);
start = end - KB(4);
continue;
}
 
-   printf(0x%08x: 0x%08x\n, start, start_phys);
+   printf(0x%08x: 0x% PRIx64 \n, start, start_phys);
}
 
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 6/7] intel_gtt: Support HSW PTEs

2013-09-01 Thread Ben Widawsky 
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 tools/intel_gtt.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/intel_gtt.c b/tools/intel_gtt.c
index 874a4f6..acf63c1 100644
--- a/tools/intel_gtt.c
+++ b/tools/intel_gtt.c
@@ -50,7 +50,10 @@ static uint64_t get_phys(uint32_t pt_offset)
if (intel_gen(devid)  6)
return phys;
 
-   pae = (phys  0xff0)  28;
+   if (IS_HASWELL(devid))
+   pae = (phys  0x7f0)  28;
+   else
+   pae = (phys  0xff0)  28;
 
return (phys | pae)  ~0xfff;
 }
-- 
1.8.4

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

[Intel-gfx] [PATCH 7/7] intel_gtt: Raw PTE dumper mode

2013-09-01 Thread Ben Widawsky 
./tools/intel_gtt -d | head
GTT offset | PTEs

  0x00 | 0xe4005015 0xe2854015 0xe283e015 0xe283f015
  0x004000 | 0xe28ba015 0xe28bb015 0xe28b6015 0xe28b7015
  0x008000 | 0xe2828015 0xe2829015 0xe282a015 0xe282b015
  0x00c000 | 0xe2928015 0xe2929015 0xe292a015 0xe292b015
  0x01 | 0xe2918015 0xe2919015 0xe291a015 0xe291b015
  0x014000 | 0xe291c015 0xe291d015 0xe291e015 0xe291f015
  0x018000 | 0xe2920015 0xe2921015 0xe2922015 0xe2923015
  0x01c000 | 0xe2924015 0xe2925015 0xe2926015 0xe2927015

Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 tools/intel_gtt.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/tools/intel_gtt.c b/tools/intel_gtt.c
index acf63c1..090e88d 100644
--- a/tools/intel_gtt.c
+++ b/tools/intel_gtt.c
@@ -58,6 +58,25 @@ static uint64_t get_phys(uint32_t pt_offset)
return (phys | pae)  ~0xfff;
 }
 
+static void pte_dump(int size, uint32_t offset) {
+   int start;
+   /* Want to print 4 ptes at a time (4b PTE assumed). */
+   if (size % 16)
+   size = (size + 16)  ~0x;
+
+
+   printf(GTT offset | PTEs\n);
+   printf(\n);
+   for (start = 0; start  size; start += KB(16)) {
+   printf(  0x%06x | 0x%08x 0x%08x 0x%08x 0x%08x\n,
+   start,
+   INGTT(start + 0x0),
+   INGTT(start + 0x1000),
+   INGTT(start + 0x2000),
+   INGTT(start + 0x3000));
+   }
+}
+
 int main(int argc, char **argv)
 {
struct pci_device *pci_dev;
@@ -105,6 +124,10 @@ int main(int argc, char **argv)
}
 
aper_size = pci_dev-regions[2].size;
+   if (argc  1  !strncmp(-d, argv[1], 2)) {
+   pte_dump(aper_size, 0);
+   return 0;
+   }
 
for (start = 0; start  aper_size; start += KB(4)) {
uint64_t start_phys = get_phys(start);
-- 
1.8.4

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

Re: [Intel-gfx] [git pull] drm fixes

2013-09-01 Thread Linus Torvalds 
On Sat, Aug 31, 2013 at 4:02 PM, Linus Torvalds
torva...@linux-foundation.org wrote:

 Any known issues with DVI on Haswell (it seems to show up as HDMI1
 as the output, but it's a DVI cable)?

With a DP cable and the same monitor, the problem doesn't seem to
occur. So it does seem to somehow be related to the HDMI1/DVI output.

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