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  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 
> Cc: Sedat Dilek 
> ---
>
> 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(>t_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-next&id=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  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  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  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 
> > ---
> >  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;
> > - 

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  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  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  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 
> > ---
> >  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 p

[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  
> 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 
> > Cc: Sedat Dilek 
> > ---
> >
> > 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(>t_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 
---
 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 
Signed-off-by: Ben Widawsky 
---
 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 
---
 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 
---
 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 
---
 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 
 #include 
 #include 
 #include 
@@ -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 
---
 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 
---
 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
 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


Re: [Intel-gfx] [PATCH] drm/i915: enable trickle feed on Haswell

2013-09-01 Thread Daniel Vetter
On Fri, Aug 30, 2013 at 08:25:52PM +0300, Ville Syrjälä wrote:
> On Fri, Aug 23, 2013 at 07:51:28PM -0300, Paulo Zanoni wrote:
> > From: Paulo Zanoni 
> > 
> > We shouldn't disable the trickle feed bits on Haswell. Our
> > documentation explicitly says the trickle feed bits of PRI_CTL and
> > CUR_CTL should not be programmed to 1, and the hardware engineer also
> > asked us to not program the SPR_CTL field to 1. Leaving the bits as 1
> > could cause underflows.
> > 
> > Reported-by: Arthur Runyan 
> > Signed-off-by: Paulo Zanoni 
> 
> Reviewed-by: Ville Syrjälä 

Queued for -next, thanks for the patch.
> 
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h  |  1 +
> >  drivers/gpu/drm/i915/intel_display.c | 10 +++---
> >  drivers/gpu/drm/i915/intel_pm.c  |  2 --
> >  drivers/gpu/drm/i915/intel_sprite.c  |  7 +--
> >  4 files changed, 13 insertions(+), 7 deletions(-)
> > 
> > Some side discussions:
> > 
> >  1 - It seems we always do read-modify-write with the PRI/SPR/CUR 
> > registers. I
> >  think this is dangerous since we may forget to disable something and keep 
> > our
> >  code in a bugged state forever. Shouldn't we try to completely set the full
> >  state of the bits from scratch every time we enable PRI/SPR/CUR?
> 
> Yeah I dislike the RMW. In my atomic branch I think I got rid of it, if
> for no other reason than less overhead. I think the reason for the RMW
> was that the whole plane handling has been spread around all over the
> place. We should just centralize it nicely as part of the "all planes
> are drm_planes" change.
> 
> The base address RMW is an open question though since it was explicitly
> made that way. But on HSW that's supposedly bugged and can revert the
> based address update from a command streamer flip. I haven't actually
> tried which way it's bugged: does it immediately revert and cause the
> display to scan out garbage until the new value is latches on the next
> vblank, or does the revert also need a vblank to latch. The latter
> option is the less dangerous since we'd need to get a vblank between
> the read and write to see garbage. I should write a test now that
> there's a hsw machine on my desk...

Yeah, the rmw is a bit uncool, and I've also forgotten what the exact
reason for merging this was - the commit message is especially unhelpful.
Imo if we need to preserve some bits in there we need to properly track
them. So I'm ok with reverting this again if it causes issues on Haswell
(or if it's just generally a mess to maintain ...).
-Daniel

> >  2 - We also have code to enable the trickle feed bits at init_clock_gating.
> >  Isn't this dangerous? Trickle fee changes the memory is read, my first 
> > guess
> >  would be that it's probably not safe to change this bit when things are
> >  enabled. Also, we lose the state of these bits when the power well is 
> > disabled,
> >  so the code in init_clock_gating really makes no sense on Haswell.
> 
> I don't recall seeing any garbage when toggling trickle feed on the fly.
> I assume that if the watermarks are sufficiently high toggling trickle
> feed should be safe, well apparently before HSW that is :)
> 
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index bbbc177..e92bb47 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -3312,6 +3312,7 @@
> >  #define   MCURSOR_PIPE_A   0x00
> >  #define   MCURSOR_PIPE_B   (1 << 28)
> >  #define   MCURSOR_GAMMA_ENABLE  (1 << 26)
> > +#define   CURSOR_TRICKLE_FEED_DISABLE  (1 << 14)
> >  #define _CURABASE  (dev_priv->info->display_mmio_offset + 0x70084)
> >  #define _CURAPOS   (dev_priv->info->display_mmio_offset + 0x70088)
> >  #define   CURSOR_POS_MASK   0x007FF
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index c64631d..d5f038e 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2077,8 +2077,10 @@ static int ironlake_update_plane(struct drm_crtc 
> > *crtc,
> > else
> > dspcntr &= ~DISPPLANE_TILED;
> >  
> > -   /* must disable */
> > -   dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
> > +   if (IS_HASWELL(dev))
> > +   dspcntr &= ~DISPPLANE_TRICKLE_FEED_DISABLE;
> > +   else
> > +   dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
> >  
> > I915_WRITE(reg, dspcntr);
> >  
> > @@ -6768,8 +6770,10 @@ static void ivb_update_cursor(struct drm_crtc *crtc, 
> > u32 base)
> > cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
> > cntl |= CURSOR_MODE_DISABLE;
> > }
> > -   if (IS_HASWELL(dev))
> > +   if (IS_HASWELL(dev)) {
> > cntl |= CURSOR_PIPE_CSC_ENABLE;
> > +   cntl &= ~CURSOR_TRICKLE_FEED_DISABLE;
> > +   }
> > I915_WRITE(CURCNTR_IVB(pipe), cntl);
> >  
> > intel_crtc->cursor_visible = 

Re: [Intel-gfx] [PATCH 03/17] drm/i915: Pin pages whilst mapping the dma-buf

2013-09-01 Thread Daniel Vetter
On Fri, Aug 30, 2013 at 03:27:47PM +0100, Damien Lespiau wrote:
> On Mon, Aug 26, 2013 at 07:50:55PM -0300, Rodrigo Vivi wrote:
> > From: Chris Wilson 
> > 
> > As we attempt to kmalloc after calling get_pages, there is a possibility
> > that the shrinker may reap the pages we just acquired. To prevent this
> > we need to increment the pages_pin_count early, so rearrange the code
> > and error paths to make it so.
> > 
> > Signed-off-by: Chris Wilson 
> 
> Reviewed-by: Damien Lespiau 

I've merged the three reviewed patches to dinq, thanks.
-Daniel
> 
> -- 
> Damien
> 
> > ---
> >  drivers/gpu/drm/i915/i915_gem_dmabuf.c | 41 
> > ++
> >  1 file changed, 22 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c 
> > b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> > index e918b05..7d5752f 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> > @@ -42,27 +42,24 @@ static struct sg_table *i915_gem_map_dma_buf(struct 
> > dma_buf_attachment *attachme
> >  
> > ret = i915_mutex_lock_interruptible(obj->base.dev);
> > if (ret)
> > -   return ERR_PTR(ret);
> > +   goto err;
> >  
> > ret = i915_gem_object_get_pages(obj);
> > -   if (ret) {
> > -   st = ERR_PTR(ret);
> > -   goto out;
> > -   }
> > +   if (ret)
> > +   goto err_unlock;
> > +
> > +   i915_gem_object_pin_pages(obj);
> >  
> > /* Copy sg so that we make an independent mapping */
> > st = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
> > if (st == NULL) {
> > -   st = ERR_PTR(-ENOMEM);
> > -   goto out;
> > +   ret = -ENOMEM;
> > +   goto err_unpin;
> > }
> >  
> > ret = sg_alloc_table(st, obj->pages->nents, GFP_KERNEL);
> > -   if (ret) {
> > -   kfree(st);
> > -   st = ERR_PTR(ret);
> > -   goto out;
> > -   }
> > +   if (ret)
> > +   goto err_free;
> >  
> > src = obj->pages->sgl;
> > dst = st->sgl;
> > @@ -73,17 +70,23 @@ static struct sg_table *i915_gem_map_dma_buf(struct 
> > dma_buf_attachment *attachme
> > }
> >  
> > if (!dma_map_sg(attachment->dev, st->sgl, st->nents, dir)) {
> > -   sg_free_table(st);
> > -   kfree(st);
> > -   st = ERR_PTR(-ENOMEM);
> > -   goto out;
> > +   ret =-ENOMEM;
> > +   goto err_free_sg;
> > }
> >  
> > -   i915_gem_object_pin_pages(obj);
> > -
> > -out:
> > mutex_unlock(&obj->base.dev->struct_mutex);
> > return st;
> > +
> > +err_free_sg:
> > +   sg_free_table(st);
> > +err_free:
> > +   kfree(st);
> > +err_unpin:
> > +   i915_gem_object_unpin_pages(obj);
> > +err_unlock:
> > +   mutex_unlock(&obj->base.dev->struct_mutex);
> > +err:
> > +   return ERR_PTR(ret);
> >  }
> >  
> >  static void i915_gem_unmap_dma_buf(struct dma_buf_attachment *attachment,
> > -- 
> > 1.8.1.4
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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


Re: [Intel-gfx] [PATCH 02/19] drm/i915: Call intel_update_watermarks() in specific place during modeset

2013-09-01 Thread Daniel Vetter
On Fri, Aug 30, 2013 at 11:49:52PM +0300, Ville Syrjälä wrote:
> On Fri, Aug 30, 2013 at 05:26:29PM -0300, Paulo Zanoni wrote:
> > 2013/8/30  :
> > > From: Ville Syrjälä 
> > >
> > > Make the call to intel_update_watermarks() just once or twice during
> > > modeset. Ideally it should happen independently when each plane gets
> > > enabled/disabled, but for now it seems better to keep it in central
> > > place. We can improve things when we get all the planes sorted out
> > > in a better way.
> > >
> > > When enabling set up the watermarks just before the pipe is enabled.
> > > And when disabling we need to wait until we've marked the crtc as
> > > inactive.
> > 
> > Why do we need to wait until we've marked the CRTC as inactive?
> > (Daniel/Ville should put the answer in the commit message)
> 
> Because the watermark compute code looks at intel_crtc->active. If we
> compute the watermarks before, the code thinks the pipe is active.
> 
> Hmm. BTW now that I look at intel_crtc_active() I start to wonder why it
> looks at the clock in the user specified mode. In fact most (maybe all?)
> of the pre-hsw watermark code is fscked up and it looks at the wrong
> mode. Sigh. Suppose I need to make a quick for all that as well...

That was a fallout from the partial modeset state recovery on boot-up -
we've ended up doing divide-by-zeros since the pipe was marked as active
but we've lacked the dotclock required to compute the new watermarks. So
for those we've just skipped this.

Since hsw still has no dotclock readout support we need to keep onto this
hack for a while longer. But adding an XXX comment here would be useful.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 0/2] vgaarb: Fixes for partial VGA opt-out

2013-09-01 Thread Daniel Vetter
On Fri, Aug 30, 2013 at 03:41:19PM +0300, Ville Syrjälä wrote:
> On Thu, Aug 15, 2013 at 04:37:47PM -0600, Alex Williamson wrote:
> > I'm trying to add support for VGA arbitration on newer Intel graphics
> > devices.  The existing code attempts to do this, but appear to have
> > not been updated since GMCH devices roamed the Earth.  On newer
> > devices like Haswell, we can disable VGA memory through an MSR on the
> > device, but we rely on the VGA arbiter to manage VGA IO using the PCI
> > COMMAND register.  In trying to unregister legacy VGA memory, I found
> > that the VGA arbiter still wanted to disable both memory and IO on
> > the device and that it forgot to actually program the device to
> > disable IO when the decoding is updated.  This series attempts to fix
> > both of those.  Thanks,
> 
> The series looks good to me.
> 
> Reviewed-by: Ville Syrjälä 

Merged to dinq with Dave's irc-ack. I'll shuffle my tree a bit so that
this will be part of my 3.12 latecomers pull request to Dave.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v3] i915: Update VGA arbiter support for newer devices

2013-09-01 Thread Daniel Vetter
On Fri, Aug 30, 2013 at 03:43:05PM +0300, Ville Syrjälä wrote:
> On Wed, Aug 28, 2013 at 09:39:08AM -0600, Alex Williamson wrote:
> > This is intended to add VGA arbiter support for Intel HD graphics on
> > Core processors.  The old GMCH registers no longer exist, so even
> > though it appears that i915 participates in VGA arbitration, it doesn't
> > work.  On Intel HD graphics we already attempt to disable VGA regions
> > of the device.  This makes registering as a VGA client unnecessary since
> > we don't intend to operate differently depending on how many VGA devices
> > are present.  We can disable VGA memory regions by clearing the memory
> > enable bit in the VGA MSR.  That only leaves VGA IO, which we update
> > the VGA arbiter to know that we don't participate in VGA memory
> > arbitration.  We also add a hook on unload to re-enable memory and
> > reinstate VGA memory arbitration.
> > 
> > Signed-off-by: Alex Williamson 
> 
> Looking good.
> 
> Reviewed-by: Ville Syrjälä 

Queued for -next, thanks for the patch.
> 
> > ---
> > 
> > v3: Use explicit LEGACY_IO | LEGACY_MEM when restoring rather than
> > LEGACY_MASK, per Ville's comments.
> > 
> > v2: I915_READ/WRITE accessors don't work in i915_disable_vga, use inb/outb
> > directly.  Also, on the driver unbind VGA enable path, acquire legacy
> > IO to re-enable VGA memory.  Correct comment.

I've added the patch changelog here to the commmit message - I kinda
prefer to keep this bit of information around ...
-Daniel

> > 
> > As with v1, this depends on "vgaarb: Fixes for partial VGA opt-out".  With
> > all patches I'm able to assign a discrete PEG VGA device to a guest and
> > execute the VBIOS w/o interference from IGD or corruption of the IGD
> > framebuffer.
> > 
> >  drivers/gpu/drm/i915/i915_dma.c  |9 ++---
> >  drivers/gpu/drm/i915/intel_display.c |   25 +
> >  2 files changed, 31 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c 
> > b/drivers/gpu/drm/i915/i915_dma.c
> > index f466980..d9cf216 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1287,9 +1287,12 @@ static int i915_load_modeset_init(struct drm_device 
> > *dev)
> >  * then we do not take part in VGA arbitration and the
> >  * vga_client_register() fails with -ENODEV.
> >  */
> > -   ret = vga_client_register(dev->pdev, dev, NULL, i915_vga_set_decode);
> > -   if (ret && ret != -ENODEV)
> > -   goto out;
> > +   if (!HAS_PCH_SPLIT(dev)) {
> > +   ret = vga_client_register(dev->pdev, dev, NULL,
> > + i915_vga_set_decode);
> > +   if (ret && ret != -ENODEV)
> > +   goto out;
> > +   }
> >  
> > intel_register_dsm_handler();
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 5fb3058..2807760 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9519,6 +9519,15 @@ static void i915_disable_vga(struct drm_device *dev)
> > outb(SR01, VGA_SR_INDEX);
> > sr1 = inb(VGA_SR_DATA);
> > outb(sr1 | 1<<5, VGA_SR_DATA);
> > +
> > +   /* Disable VGA memory on Intel HD */
> > +   if (HAS_PCH_SPLIT(dev)) {
> > +   outb(inb(VGA_MSR_READ) & ~VGA_MSR_MEM_EN, VGA_MSR_WRITE);
> > +   vga_set_legacy_decoding(dev->pdev, VGA_RSRC_LEGACY_IO |
> > +  VGA_RSRC_NORMAL_IO |
> > +  VGA_RSRC_NORMAL_MEM);
> > +   }
> > +
> > vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
> > udelay(300);
> >  
> > @@ -9526,6 +9535,20 @@ static void i915_disable_vga(struct drm_device *dev)
> > POSTING_READ(vga_reg);
> >  }
> >  
> > +static void i915_enable_vga(struct drm_device *dev)
> > +{
> > +   /* Enable VGA memory on Intel HD */
> > +   if (HAS_PCH_SPLIT(dev)) {
> > +   vga_get_uninterruptible(dev->pdev, VGA_RSRC_LEGACY_IO);
> > +   outb(inb(VGA_MSR_READ) | VGA_MSR_MEM_EN, VGA_MSR_WRITE);
> > +   vga_set_legacy_decoding(dev->pdev, VGA_RSRC_LEGACY_IO |
> > +  VGA_RSRC_LEGACY_MEM |
> > +  VGA_RSRC_NORMAL_IO |
> > +  VGA_RSRC_NORMAL_MEM);
> > +   vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
> > +   }
> > +}
> > +
> >  void intel_modeset_init_hw(struct drm_device *dev)
> >  {
> > intel_init_power_well(dev);
> > @@ -9983,6 +10006,8 @@ void intel_modeset_cleanup(struct drm_device *dev)
> >  
> > intel_disable_fbc(dev);
> >  
> > +   i915_enable_vga(dev);
> > +
> > intel_disable_gt_powersave(dev);
> >  
> > ironlake_teardown_rc6(dev);
> 
> -- 
> Ville Syrjälä
> Intel OTC
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/m

Re: [Intel-gfx] [PATCH] drm/i915: Don't call sg_free_table() if sg_alloc_table() fails

2013-09-01 Thread Daniel Vetter
On Fri, Aug 30, 2013 at 05:00:55PM +0100, Chris Wilson wrote:
> On Fri, Aug 30, 2013 at 03:39:26PM +0100, Damien Lespiau wrote:
> > One needs to call __sg_free_table() if __sg_alloc_table() fails, but
> > sg_alloc_table() does that for us already.
> > 
> > Signed-off-by: Damien Lespiau 
> Reviewd-by: Chris Wilson 

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


Re: [Intel-gfx] [PATCH] drm/i915: It's its!

2013-09-01 Thread Daniel Vetter
On Fri, Aug 30, 2013 at 03:26:30PM -0700, Ben Widawsky wrote:
> On Fri, Aug 30, 2013 at 02:40:26PM +0100, Damien Lespiau wrote:
> > Signed-off-by: Damien Lespiau 
> > ---
> >  drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
> > b/drivers/gpu/drm/i915/i915_gem_context.c
> > index 403309c..e038513 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> > @@ -73,7 +73,7 @@
> >   *
> >   * There are two confusing terms used above:
> >   *  The "current context" means the context which is currently running on 
> > the
> > - *  GPU. The GPU has loaded it's state already and has stored away the gtt
> > + *  GPU. The GPU has loaded its state already and has stored away the gtt
> >   *  offset of the BO. The GPU is not actively referencing the data at this
> >   *  offset, but it will on the next context switch. The only way to avoid 
> > this
> >   *  is to do a GPU reset.
> 
> Acked-by: Ben Widawsky 

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


Re: [Intel-gfx] [PATCH 1/8] drm/i915: Synchronize pread/pwrite with wait_rendering

2013-09-01 Thread Daniel Vetter
On Fri, Aug 30, 2013 at 08:39:46PM -0700, Ben Widawsky wrote:
> On Sat, Aug 31, 2013 at 12:50:30AM +0100, Chris Wilson wrote:
> > On Fri, Aug 30, 2013 at 04:43:54PM -0700, Ben Widawsky wrote:
> > > lifted from Daniel:
> > > pread/pwrite isn't about the object's domain at all, but purely about
> > > synchronizing for outstanding rendering. Replacing the call to
> > > set_to_gtt_domain with a wait_rendering would imo improve code
> > > readability. Furthermore we could pimp pread to only block for
> > > outstanding writes and not for reads.
> > > 
> > > Since you're not the first one to trip over this: Can I volunteer you
> > > for a follow-up patch to fix this?
> > > 
> > > Recommended-by: Daniel Vetter 
> > > Signed-off-by: Ben Widawsky 
> > 
> > This should fail i-g-t...
> > -Chris
> > 
> 
> Daniel, how have I failed your plan?

It should work ... Since the enclosing if-block checks for !cpu domain
(for either reads or writes) that implies that going into the gtt domain
is a noop (or better should be) wrt clflushing and we only wait for
outstanding gpu rendering. wait_rendering is an interface that's been
added afterwards. Unfortunately I've failed to explain this trickery in
either a comment or the commit message. Bad me ;-)

What does QA's patch test system say on a non-llc machine here?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] Avoid i915 flip unpin/HPD event handler deadlock.

2013-09-01 Thread Daniel Vetter
On Sat, Aug 31, 2013 at 10:15:25AM +0100, Chris Wilson wrote:
> On Fri, Aug 30, 2013 at 06:30:55PM -0700, Stuart Abercrombie wrote:
> > Both of these were taking the mode_config mutex but executed from the
> > same work queue.  If intel_crtc_page_flip happened to flush a work queue
> > containing an HPD event handler work item, deadlock resulted, since the
> > mutex required by the HPD handler was taken before the flush.  Instead
> > use a separate work queue for the flip unpin work.
> > 
> > Signed-off-by: sabercrom...@chromium.org
> > Reviewed-by: marc...@chromium.org
> 
> It would be possible to rearrange the flip to drop the lock around the
> flush (which is a regression from the kernel/workqueue.c refacting...).
> However, this looks much simpler. In the long run being strict on
> calling flush_workqueue() unlocked is likely to be safer though.
> 
> Reviewed-by; Chris Wilson 

Actually this is a regression from

commit 69787f7da6b2adc4054357a661aaa1701a9ca76f
Author: Daniel Vetter 
Date:   Tue Oct 23 18:23:34 2012 +

drm: run the hpd irq event code directly

and the fix is a bit simpler.
-Daniel

> 
> > ---
> >  drivers/gpu/drm/i915/i915_dma.c  | 21 -
> >  drivers/gpu/drm/i915/i915_drv.h  |  1 +
> >  drivers/gpu/drm/i915/intel_display.c |  4 ++--
> >  3 files changed, 23 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c 
> > b/drivers/gpu/drm/i915/i915_dma.c
> > index 4f129bb..9215360 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1558,6 +1558,22 @@ int i915_driver_load(struct drm_device *dev, 
> > unsigned long flags)
> > goto out_mtrrfree;
> > }
> >  
> > +   /* intel_crtc_page_flip runs with the mode_config mutex having been
> > +* taken in the DRM layer.  It synchronously waits for pending unpin
> > +* work items while holding this mutex.  Therefore this queue cannot
> > +* contain work items that take this mutex, such as HPD event
> > +* handling, or we deadlock.  There is also no reason for flipping to
> > +* wait on such events.  Therefore put flip unpinning in its own
> > +* work queue.
> > +*/
> > +   dev_priv->flip_unpin_wq = alloc_ordered_workqueue("i915", 0);
> > +   if (dev_priv->flip_unpin_wq == NULL) {
> > +   DRM_ERROR("Failed to create flip unpin workqueue.\n");
> > +   destroy_workqueue(dev_priv->wq);
> > +   ret = -ENOMEM;
> > +   goto out_mtrrfree;
> > +   }
> > +
> > /* This must be called before any calls to HAS_PCH_* */
> > intel_detect_pch(dev);
> >  
> > @@ -1628,6 +1644,7 @@ out_gem_unload:
> > intel_teardown_gmbus(dev);
> > intel_teardown_mchbar(dev);
> > destroy_workqueue(dev_priv->wq);
> > +   destroy_workqueue(dev_priv->flip_unpin_wq);
> 
> To be consistent, flip_wq then wq. In case we get ordering issues later.
> -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

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


[Intel-gfx] [PATCH] drm/i915: fix hpd work vs. flush_work in the pageflip code deadlock

2013-09-01 Thread Daniel Vetter
Historically we've run our own driver hotplug handling in our own
work-queue, which then launched the drm core hotplug handling in the
system workqueue. This is important since we flush our own driver
workqueue in the pageflip code while hodling modeset locks, and only
the drm hotplug code grabbed these locks. But with

commit 69787f7da6b2adc4054357a661aaa1701a9ca76f
Author: Daniel Vetter 
Date:   Tue Oct 23 18:23:34 2012 +

drm: run the hpd irq event code directly

this was changed and now we could deadlock in our flip handler if
there's a hotplug work blocking the progress of the crucial unpin
works. So this broke the careful deadlock avoidance implemented in

commit b4a98e57fc27854b5938fc8b08b68e5e68b91e1f
Author: Chris Wilson 
Date:   Thu Nov 1 09:26:26 2012 +

drm/i915: Flush outstanding unpin tasks before pageflipping

Since the rule thus far has been that work items on our own workqueue
may never grab modeset locks simply restore that rule again.

Cc: Chris Wilson 
Cc: Stuart Abercrombie 
Reported-by: Stuart Abercrombie 
References: 
http://permalink.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/26239
Cc: sta...@vger.kernel.org
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_irq.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 4c6853f..39e4dd0 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1027,8 +1027,7 @@ static inline void intel_hpd_irq_handler(struct 
drm_device *dev,
dev_priv->display.hpd_irq_setup(dev);
spin_unlock(&dev_priv->irq_lock);
 
-   queue_work(dev_priv->wq,
-  &dev_priv->hotplug_work);
+   schedule_work(&dev_priv->hotplug_work);
 }
 
 static void gmbus_irq_handler(struct drm_device *dev)
-- 
1.8.4.rc3

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


Re: [Intel-gfx] BUG: i830 flickering for horizontal panning

2013-09-01 Thread Daniel Vetter
On Sat, Aug 31, 2013 at 05:40:09PM +0200, Thomas Richter wrote:
> Dear intel-gfx developers,
> 
> panning on i830 based graphics seem to be working only half-ways.
> Vertical panning works fine, but horizontal panning flickers at
> about 60Hz frequency at specific pixel positions. The problem
> persists throughout kernel 3.10.9, and potentially later.
> 
> How to reproduce: Enable panning, e.g. by
> 
> xrandr --fb 2048x1536
> xrandr --output DVI1 --panning 2048x1536
> 
> then scroll with the mouse slowly to the right.
> 
> The affected hardware is *at least* this one:
> 
> 00:02.0 VGA compatible controller: Intel Corporation 82830M/MG
> Integrated Graphics Controller (rev 04)
> 
> as it is found in the Fujitsu-Siemens S2475 lifebook. The IBM
> Thinkpad R31 is also affected, it also contains a 830GM chipset
> graphics, but a different DVO.
> 
> There is no specific output when flickering. The screen contents
> seems to be flickering between a dark frame and a frame that is half
> the way shifted correctly (left) and half the way a copy of the
> leftmost pixels of the screen on the right hand side.
> 
> The kernel uses the i915 driver for this hardware.
> 
> I currently don't see where precisely panning in the i915 sources is
> handled, thus I cannot really say what is necessary to support this
> correctly.

Is this a regression?

For your other bug reports we know that the modesetting support for i830M
chipsets is a bit in a sorry state of affairs, search for 830 at
bugs.freedesktop.org. Unfortunately time to really dig into this issues is
hard to come by :(
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2013-09-01 Thread Daniel Vetter
On Sat, Aug 31, 2013 at 04:02:16PM -0700, Linus Torvalds wrote:
> Hmm. I just updated my machine to a i7-4770S (kept everything else the
> same, just switched out motherboards), and now when my display goes to
> sleep, it seems to never come back.

Sleep as in dpms off ($ xset dpms force off) or sleep as in system
suspend?

> Any known issues with DVI on Haswell (it seems to show up as "HDMI1"
> as the output, but it's a DVI cable)? I need to get a DP cable anyway
> (right now I'm driving my 2560x1440 display at 32Hz due to DVI crap)
> and maybe that will fix things, but this happens even if I don't force
> that odd mode (so it maxes out at 1920x1200 or whatever).
> 
> I doubt this is new, but I've only ever run current -git on this
> machine, so who knows..  The machine ends up being kind of unusable. I
> guess I can just turn off power save.

Hm, doesn't ring a bell here. Adding Paulo, our residential hsw display
expert.

Aside hsw hdmi ports support 300MHz dotclocks, but still only single-link.
So I guess your dvi screen is still out of luck since it likely wants a
dual-link dvi signal. But if you have a hdmi port it should work a bit
better (hdmi pumped up the max dotclock a bit already a while ago ...).

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