Re: [Intel-gfx] [PATCH 3/3] drm/i915: re-enable rc6 by default when GMAR is disabled

2011-11-16 Thread Ben Widawsky
On Wed, 16 Nov 2011 22:11:06 -0800
keith.pack...@intel.com wrote:

> On Wed, 16 Nov 2011 16:56:16 -0800, Ben Widawsky  wrote:
> 
> > The variable you want is: !intel_iommu_gfx_mapped
> 
> From what Daniel Vetter said:
> 
> > Last time I've played around with all the combinations, only
> > intel_iommu=off was good enough to prevent the hang.
> > intel_iommu=igd_off still hangs (which is what the current value
> > exported by the dmar code dopends on, iirc). If I remember things
> > correctly, intel_iommu=off also reliably works around issues for all
> > reporters (both semaphores and rc6).
> 
> we need to use dmar_disabled instead of intel_iommu_gfx_mapped. Other
> places in the code use:
> 
> if (no_iommu || dmar_disabled)
> 

As long as he meant igfx_off, then okay... igd_off didn't immediately register
to me earlier. Sorry for the confusion.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [RFC PATCH] drm: Fix off-by-one races on vblank disable

2011-11-16 Thread Andy Lutomirski
There are two possible races when disabling vblanks.  If the IRQ
fired but the hardware didn't update its counter yet, then we store
too low a hardware counter.  (Sensible hardware never does this.
Apparently not all hardware is sensible.)  If, on the other hand,
the counter updated but the IRQ didn't fire yet, we store too high a
counter.

We handled the former case with a heuristic based on timestamps and
we did not handle the latter case.  By saving a little more state,
we can handle both cases exactly: all we need to do is watch for
changes in the difference between the hardware and software vblank
counts.

Signed-off-by: Andy Lutomirski 
---

Rather than tweaking more things to reduce the chance of hitting a race
while keeping the vblank disable timeout as low as possible, why not
just fix the race?

This compiles but is not very well tested, because I don't know what
tests to run.  I haven't been able to provoke either race on my SNB
laptop.

 drivers/gpu/drm/drm_irq.c |   92 
 include/drm/drmP.h|2 +-
 2 files changed, 59 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 3830e9e..1674a33 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -56,6 +56,12 @@
  */
 #define DRM_REDUNDANT_VBLIRQ_THRESH_NS 100
 
+/* Saved vblank count data, used only in this file. */
+struct drm_vbl_counts {
+   u32 hwcount;/* hw count at last state save or load */
+   u32 drmcount;   /* drm count at last state save or load */
+};
+
 /**
  * Get interrupt from bus id.
  *
@@ -101,7 +107,8 @@ static void clear_vblank_timestamps(struct drm_device *dev, 
int crtc)
 static void vblank_disable_and_save(struct drm_device *dev, int crtc)
 {
unsigned long irqflags;
-   u32 vblcount;
+   u32 drmcount, hwcount;
+   u32 drm_counts_seen, hw_counts_seen, offset;
s64 diff_ns;
int vblrc;
struct timeval tvblank;
@@ -121,44 +128,53 @@ static void vblank_disable_and_save(struct drm_device 
*dev, int crtc)
/* No further vblank irq's will be processed after
 * this point. Get current hardware vblank count and
 * vblank timestamp, repeat until they are consistent.
-*
-* FIXME: There is still a race condition here and in
-* drm_update_vblank_count() which can cause off-by-one
-* reinitialization of software vblank counter. If gpu
-* vblank counter doesn't increment exactly at the leading
-* edge of a vblank interval, then we can lose 1 count if
-* we happen to execute between start of vblank and the
-* delayed gpu counter increment.
 */
do {
-   dev->last_vblank[crtc] = dev->driver->get_vblank_counter(dev, 
crtc);
+   hwcount = dev->driver->get_vblank_counter(dev, crtc);
vblrc = drm_get_last_vbltimestamp(dev, crtc, &tvblank, 0);
-   } while (dev->last_vblank[crtc] != dev->driver->get_vblank_counter(dev, 
crtc));
+   } while (hwcount != dev->driver->get_vblank_counter(dev, crtc));
 
/* Compute time difference to stored timestamp of last vblank
 * as updated by last invocation of drm_handle_vblank() in vblank irq.
 */
-   vblcount = atomic_read(&dev->_vblank_count[crtc]);
+   drmcount = atomic_read(&dev->_vblank_count[crtc]);
diff_ns = timeval_to_ns(&tvblank) -
- timeval_to_ns(&vblanktimestamp(dev, crtc, vblcount));
+ timeval_to_ns(&vblanktimestamp(dev, crtc, drmcount));
 
-   /* If there is at least 1 msec difference between the last stored
-* timestamp and tvblank, then we are currently executing our
-* disable inside a new vblank interval, the tvblank timestamp
-* corresponds to this new vblank interval and the irq handler
-* for this vblank didn't run yet and won't run due to our disable.
-* Therefore we need to do the job of drm_handle_vblank() and
-* increment the vblank counter by one to account for this vblank.
+   /* We could be off by one in either direction.  If a vblank just
+* happened but the IRQ hasn't been handled yet, then drmcount is
+* too low by one.  On the other hand, if the GPU fires its vblank
+* interrupts *before* updating its counter, then hwcount could
+* be too low by one.  (If both happen, they cancel out.)
 *
-* Skip this step if there isn't any high precision timestamp
-* available. In that case we can't account for this and just
-* hope for the best.
+* Fortunately, we have enough information to figure out what
+* happened.  Assuming the hardware counter works right, the
+* difference between drmcount and vblcount should be a constant
+* (modulo max_vblank_count).  We have both saved values from last
+* time we turned the interrupt on.
 */
- 

Re: [Intel-gfx] [RFC] Reduce idle vblank wakeups

2011-11-16 Thread Andrew Lutomirski
On Wed, Nov 16, 2011 at 6:19 PM, Matthew Garrett  wrote:
> On Thu, Nov 17, 2011 at 01:26:37AM +0100, Mario Kleiner wrote:
>> On Nov 16, 2011, at 7:48 PM, Matthew Garrett wrote:
>> >For Radeon, I'd have thought you could handle this by scheduling
>> >an irq
>> >for the beginning of scanout (avivo has a register for that) and
>> >delaying the vblank disable until you hit it?
>>
>> For Radeon there is such an irq, but iirc we had some discussions on
>> this, also with Alex Deucher, a while ago and some irq's weren't
>> considered very reliable, or already used for other stuff. The idea
>> i had goes like this:
>>
>> Use the crtc scanout position queries together with the vblank
>> counter queries inside some calibration loop, maybe executed after
>> each modeset, to find out the scanline range in which the hardware
>> vblank counter increments -- basically a forbidden range of scanline
>> positions where the race would happen. Then at each vblank off/on,
>> query scanout position before and after the hw vblank counter query.
>> If according to the scanout positions the vblank counter query
>> happened within the forbidden time window, retry the query. With a
>> well working calibration that should add no delay in most cases and
>> a delay to the on/off code of a few dozen microseconds (=duration of
>> a few scanlines) worst case.
>
> Assuming we're sleeping rather than busy-looping, that's certainly ok.
> My previous experiments with radeon indicated that the scanout irq was
> certainly not entirely reliable - on the other hand, I was trying to use
> it for completing memory reclocking within the vblank interval. It was
> typically still within a few scanlines, so a sanity check there wouldn't
> pose too much of a problem.

I think there's a simpler fix: just keep the hardware and software
counts in sync -- if everything is working correctly (even with all
these crazy races), the difference should be constant.  Patch coming
momentarily.

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


Re: [Intel-gfx] [PATCH 0/2] Introduce Glamor to UXA framework.

2011-11-16 Thread Zhigang Gong
> -Original Message-
> From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> Sent: Thursday, November 17, 2011 9:14 AM
> To: Zhigang Gong; intel-gfx@lists.freedesktop.org
> Cc: zhigang.g...@linux.intel.com
> Subject: Re: [PATCH 0/2] Introduce Glamor to UXA framework.
> 
> On Wed, 16 Nov 2011 15:04:35 +0800, Zhigang Gong
>  wrote:
> > This patchset initially enable glamor with UXA. And two functions
> > ,fill_spans and poly_fill_rects, go to the glamor path. I tested it
> > with render check, and it works fine.
> 
> I split your patches slightly differently and pushed them.

I just checked out the master branch, and have a simple test, everything
seems
ok. Thanks.

> I could only verify
> that it didn't impact UXA without the glamor_egl module available. Do you
> have a patch for testing?

Now glamor was extracted from the xorg and be a separate library. Please
checkout glamor at : git://people.freedesktop.org/~gongzg/glamor.
This library implements two modules glamor and glamor_egl which are
required for intel video driver to enable glamor.

> -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] [RFC] Reduce idle vblank wakeups

2011-11-16 Thread Matthew Garrett
On Thu, Nov 17, 2011 at 01:26:37AM +0100, Mario Kleiner wrote:
> On Nov 16, 2011, at 7:48 PM, Matthew Garrett wrote:
> >I'll admit that I'm struggling to understand the issue here. If the
> >vblank counter is incremented at the time of vblank (which isn't the
> >case for radeon, it seems, but as far as I can tell is the case for
> >Intel) then how does ping-ponging the IRQ matter?
> >vblank_disable_and_save() appears to handle this case.
> >
> 
> The drm software vblank counter which is used for scheduling swaps
> etc. gets incremented in the gpu's vblank irq handler. The gpu's
> hardware counter gets incremented somewhere inside the vblank
> interval. The increments don't happen at the same point in time. The
> race is between the vblank on/off code, which gets scheduled either
> by the timeout timer for the "off" case, or by usercode for the "on"
> case and the gpu's hardware vblank counter.

Yes, that makes sense given the current design.

> The existing code uses a lock (vblank_time_lock) to prevent some
> races between itself and the vblank irq handler, but it can't "lock"
> the gpu, so if the enable/disable code executes while the gpu is
> scanning out the vblank interval, it is undefined if the final
> vblank count will be correct or off by one. Vblank duration is
> somewhere up to 4.5% of refresh interval duration, so there's your
> up to 4% chance of getting it wrong.

Well presumably by "undefined" we really mean "hardware-specific" - for 
any given well-functioning hardware I'd expect the answer to be 
well-defined. Like I said, with Radeon we have the option of triggering 
an interrupt at the point where the hardware counter is actually 
incremented. If that then shuts down the vblank interrupt then we should 
have a well-defined answer.

> >Does the timeout serve any purpose other than letting software
> >effectively prevent vblanks from being disabled?
> 
> With perfect drivers and gpu's in a perfect world, no. In reality
> there's the race i described above, and nouveau and all other
> drivers except intel and radeon. The vblank irq also drives
> timestamping of vblanks, one update per vblank. The timestamps are
> cached if a client needs them inbetween updates. Turning off vblank
> irq invalidates the timestamps. radeon and intel can recreate the
> timestamp anytime as needed, but nouveau lacks this atm., so
> timestamps remain invalid for a whole video refresh cycle after
> vblank irq on. We have patches for nouveau kms almost ready, so only
> the race mentioned above would remain.

Sure. We'd certainly need to improve things for other GPUs before 
enabling the same functionality there.

> >For Radeon, I'd have thought you could handle this by scheduling
> >an irq
> >for the beginning of scanout (avivo has a register for that) and
> >delaying the vblank disable until you hit it?
> 
> For Radeon there is such an irq, but iirc we had some discussions on
> this, also with Alex Deucher, a while ago and some irq's weren't
> considered very reliable, or already used for other stuff. The idea
> i had goes like this:
> 
> Use the crtc scanout position queries together with the vblank
> counter queries inside some calibration loop, maybe executed after
> each modeset, to find out the scanline range in which the hardware
> vblank counter increments -- basically a forbidden range of scanline
> positions where the race would happen. Then at each vblank off/on,
> query scanout position before and after the hw vblank counter query.
> If according to the scanout positions the vblank counter query
> happened within the forbidden time window, retry the query. With a
> well working calibration that should add no delay in most cases and
> a delay to the on/off code of a few dozen microseconds (=duration of
> a few scanlines) worst case.

Assuming we're sleeping rather than busy-looping, that's certainly ok. 
My previous experiments with radeon indicated that the scanout irq was 
certainly not entirely reliable - on the other hand, I was trying to use 
it for completing memory reclocking within the vblank interval. It was 
typically still within a few scanlines, so a sanity check there wouldn't 
pose too much of a problem.

> >I've no problem with all of this work being case by case.
> 
> Me neither. I just say if you'd go to the extreme and disable vblank
> irq's immediately with zero delay, without reliably fixing the
> problem i mentioned, you'd get those off by one counts, which would
> be very bad for apps that rely on precise timing. Doing so would
> basically make the whole oml_sync_control implementation mostly
> useless.

Right. My testing of sandybridge suggests that there wasn't a problem 
here - even with the ping-ponging I was reliably getting 60 interrupts 
in 60 seconds, with the counter incrementing by 1 each time. I certainly 
wouldn't push to enable it elsewhere without making sure that the 
results are reliable.

> >If vblanks are disabled and then re-enabled between 1 and 3,
> >what's the

Re: [Intel-gfx] [PATCH 3/3] drm/i915: re-enable rc6 by default when GMAR is disabled

2011-11-16 Thread Ben Widawsky
On Wed, Nov 16, 2011 at 11:02:36PM -0200, Eugeni Dodonov wrote:
> On Wed, Nov 16, 2011 at 22:56, Ben Widawsky  wrote:
> 
> > This is not sufficient. You need to know that DMAR is compiled in, and
> > is actually being used.
> >
> > The variable you want is: !intel_iommu_gfx_mapped
> >
> > I think I saw Keith say he was sending this patch out on IRC.
> >
> 
> Thanks, makes sense!
> 
> I'll wait for Keith patch then to avoid duplicating work.
> 
> On a side note, would it make sense to add the .enable_rc6 bits to
> i915_drv.c, in a similar way I did for semaphores?

I'm pretty sure we used to have that. In general, I dislike the idea of
device specific parameters until we know exactly why something doesn't
work. It makes it too tempting to just something off instead of
debugging it.
In your patch series I would have changed the check to be
if (GEN7)
do semaphore stuff
For the first patch, and then for the second patch
if (>= GEN6)
do semaphore stuff

Instead of changing the module parameter.

But that's just me, and others tend to disagree with me more than most.

Only bikeshedding because you asked :-).

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


Re: [Intel-gfx] [PATCH 0/2] Introduce Glamor to UXA framework.

2011-11-16 Thread Chris Wilson
On Wed, 16 Nov 2011 15:04:35 +0800, Zhigang Gong  
wrote:
> This patchset initially enable glamor with UXA. And two functions
> ,fill_spans and poly_fill_rects, go to the glamor path. I tested it
> with render check, and it works fine.

I split your patches slightly differently and pushed them. I could
only verify that it didn't impact UXA without the glamor_egl module
available. Do you have a patch for testing?
-Chrid

-- 
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 3/3] drm/i915: re-enable rc6 by default when GMAR is disabled

2011-11-16 Thread Eugeni Dodonov
On Wed, Nov 16, 2011 at 22:56, Ben Widawsky  wrote:

> This is not sufficient. You need to know that DMAR is compiled in, and
> is actually being used.
>
> The variable you want is: !intel_iommu_gfx_mapped
>
> I think I saw Keith say he was sending this patch out on IRC.
>

Thanks, makes sense!

I'll wait for Keith patch then to avoid duplicating work.

On a side note, would it make sense to add the .enable_rc6 bits to
i915_drv.c, in a similar way I did for semaphores?

-- 
Eugeni Dodonov

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


Re: [Intel-gfx] [PATCH 2/3] drm/i915: allow semaphores on SNB if DMAR is disabled

2011-11-16 Thread Ben Widawsky
On Wed, Nov 16, 2011 at 10:17:54PM -0200, Eugeni Dodonov wrote:
> Semaphores cause issues when DMAR is enabled. So if we are set to per-chip
> default, and we are on SNB, we can enable semaphores as long as SMAR is
> disabled.
> 
> Signed-off-by: Eugeni Dodonov 

See my response to patch 3...
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 3/3] drm/i915: re-enable rc6 by default when GMAR is disabled

2011-11-16 Thread Ben Widawsky
On Wed, Nov 16, 2011 at 10:17:55PM -0200, Eugeni Dodonov wrote:
> Most of the rc6-related hangs and major issues were addressed for the past
> months.
> 
> Let's re-enable it by default to provide a more wider testing, and catch
> the remaining problems.
> 
> According to tests, enablement of rc6 results in up to +50% improvements
> in power usage and battery life, so it certainly would be a nice feature
> to have enabled by default.
> 
> Also, most of the rc6-related issues seem to came from GMAR, so we only
> enable it as long as GMAR is disabled.
> 
> CC: Keith Packard 
> CC: Daniel Vetter 
> CC: Jesse Barnes 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38567
> Signed-off-by: Eugeni Dodonov 
> ---
>  drivers/gpu/drm/i915/i915_drv.c  |2 +-
>  drivers/gpu/drm/i915/intel_display.c |3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 565725c..337a568 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -63,7 +63,7 @@ module_param_named(semaphores, i915_semaphores, int, 0600);
>  MODULE_PARM_DESC(semaphores,
>   "Use semaphores for inter-ring sync (default: -1 (use per-chip 
> defaults))");
>  
> -unsigned int i915_enable_rc6 __read_mostly = 0;
> +unsigned int i915_enable_rc6 __read_mostly = 1;
>  module_param_named(i915_enable_rc6, i915_enable_rc6, int, 0600);
>  MODULE_PARM_DESC(i915_enable_rc6,
>   "Enable power-saving render C-state 6 (default: true)");
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 981b1f1..5dd0878 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -31,6 +31,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include "drmP.h"
>  #include "intel_drv.h"
> @@ -8746,7 +8747,7 @@ void intel_modeset_init(struct drm_device *dev)
>  
>  void intel_modeset_gem_init(struct drm_device *dev)
>  {
> - if (IS_IRONLAKE_M(dev))
> + if (IS_IRONLAKE_M(dev) && dmar_disabled)
>   ironlake_enable_rc6(dev);
>  
>   intel_setup_overlay(dev);
> -- 

This is not sufficient. You need to know that DMAR is compiled in, and
is actually being used.

The variable you want is: !intel_iommu_gfx_mapped

I think I saw Keith say he was sending this patch out on IRC.

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


[Intel-gfx] [PATCH] drm/i915: Hook up Ivybridge eDP

2011-11-16 Thread Keith Packard
The Ivybridge eDP control register looks like a cross between a
Cougarpoint PCH DP control register and a Sandybridge eDP control
register.

Where things trivially match, share the code. Where there are any
tricky bits, just split things out into two obviously separate code paths.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/i915_reg.h |   18 +
 drivers/gpu/drm/i915/intel_dp.c |  151 ++-
 2 files changed, 135 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b080cc8..43f27ad 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3447,6 +3447,24 @@
 #define  EDP_LINK_TRAIN_800_1200MV_0DB_SNB_B   (0x38<<22)
 #define  EDP_LINK_TRAIN_VOL_EMP_MASK_SNB   (0x3f<<22)
 
+/* IVB */
+#define EDP_LINK_TRAIN_400MV_0DB_IVB   (0x24 <<22)
+#define EDP_LINK_TRAIN_400MV_3_5DB_IVB (0x2a <<22)
+#define EDP_LINK_TRAIN_400MV_6DB_IVB   (0x2f <<22)
+#define EDP_LINK_TRAIN_600MV_0DB_IVB   (0x30 <<22)
+#define EDP_LINK_TRAIN_600MV_3_5DB_IVB (0x36 <<22)
+#define EDP_LINK_TRAIN_800MV_0DB_IVB   (0x38 <<22)
+#define EDP_LINK_TRAIN_800MV_3_5DB_IVB (0x33 <<22)
+
+/* legacy values */
+#define EDP_LINK_TRAIN_500MV_0DB_IVB   (0x00 <<22)
+#define EDP_LINK_TRAIN_1000MV_0DB_IVB  (0x20 <<22)
+#define EDP_LINK_TRAIN_500MV_3_5DB_IVB (0x02 <<22)
+#define EDP_LINK_TRAIN_1000MV_3_5DB_IVB(0x22 <<22)
+#define EDP_LINK_TRAIN_1000MV_6DB_IVB  (0x23 <<22)
+
+#define  EDP_LINK_TRAIN_VOL_EMP_MASK_IVB   (0x3f<<22)
+
 #define  FORCEWAKE 0xA18C
 #define  FORCEWAKE_ACK 0x130090
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ec28aeb..f63c6b2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -361,8 +361,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 * clock divider.
 */
if (is_cpu_edp(intel_dp)) {
-   if (IS_GEN6(dev))
-   aux_clock_divider = 200; /* SNB eDP input clock at 
400Mhz */
+   if (IS_GEN6(dev) || IS_GEN7(dev))
+   aux_clock_divider = 200; /* SNB & IVB eDP input clock 
at 400Mhz */
else
aux_clock_divider = 225; /* eDP input clock at 450Mhz */
} else if (HAS_PCH_SPLIT(dev))
@@ -816,10 +816,11 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct 
drm_display_mode *mode,
}
 
/*
-* There are three kinds of DP registers:
+* There are four kinds of DP registers:
 *
 *  IBX PCH
-*  CPU
+*  SNB CPU
+*  IVB CPU
 *  CPT PCH
 *
 * IBX PCH and CPU are the same for almost everything,
@@ -872,7 +873,26 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct 
drm_display_mode *mode,
 
/* Split out the IBX/CPU vs CPT settings */
 
-   if (!HAS_PCH_CPT(dev) || is_cpu_edp(intel_dp)) {
+   if (is_cpu_edp(intel_dp) && IS_GEN7(dev)) {
+   if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
+   intel_dp->DP |= DP_SYNC_HS_HIGH;
+   if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
+   intel_dp->DP |= DP_SYNC_VS_HIGH;
+   intel_dp->DP |= DP_LINK_TRAIN_OFF_CPT;
+
+   if (intel_dp->link_configuration[1] & 
DP_LANE_COUNT_ENHANCED_FRAME_EN)
+   intel_dp->DP |= DP_ENHANCED_FRAMING;
+
+   intel_dp->DP |= intel_crtc->pipe << 29;
+
+   /* don't miss out required setting for eDP */
+   intel_dp->DP |= DP_PLL_ENABLE;
+   if (adjusted_mode->clock < 20)
+   intel_dp->DP |= DP_PLL_FREQ_160MHZ;
+   else
+   intel_dp->DP |= DP_PLL_FREQ_270MHZ;
+   
+   } else if (!HAS_PCH_CPT(dev) || is_cpu_edp(intel_dp)) {
intel_dp->DP |= intel_dp->color_range;
 
if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
@@ -1374,34 +1394,60 @@ static char *link_train_names[] = {
  * These are source-specific values; current Intel hardware supports
  * a maximum voltage of 800mV and a maximum pre-emphasis of 6dB
  */
-#define I830_DP_VOLTAGE_MAXDP_TRAIN_VOLTAGE_SWING_800
-#define I830_DP_VOLTAGE_MAX_CPTDP_TRAIN_VOLTAGE_SWING_1200
 
 static uint8_t
-intel_dp_pre_emphasis_max(uint8_t voltage_swing)
+intel_dp_voltage_max(struct intel_dp *intel_dp)
 {
-   switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
-   case DP_TRAIN_VOLTAGE_SWING_400:
-   return DP_TRAIN_PRE_EMPHASIS_6;
-   case DP_TRAIN_VOLTAGE_SWING_600:
-   return DP_TRAIN_PRE_EMPHASIS_6;
-   case DP_TRAIN_VOLTAGE_SWING_800:
-   return DP_TRAIN_PRE_EMPHASIS_3_5;
-   case DP_

[Intel-gfx] [PATCH 3/3] drm/i915: re-enable rc6 by default when GMAR is disabled

2011-11-16 Thread Eugeni Dodonov
Most of the rc6-related hangs and major issues were addressed for the past
months.

Let's re-enable it by default to provide a more wider testing, and catch
the remaining problems.

According to tests, enablement of rc6 results in up to +50% improvements
in power usage and battery life, so it certainly would be a nice feature
to have enabled by default.

Also, most of the rc6-related issues seem to came from GMAR, so we only
enable it as long as GMAR is disabled.

CC: Keith Packard 
CC: Daniel Vetter 
CC: Jesse Barnes 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38567
Signed-off-by: Eugeni Dodonov 
---
 drivers/gpu/drm/i915/i915_drv.c  |2 +-
 drivers/gpu/drm/i915/intel_display.c |3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 565725c..337a568 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -63,7 +63,7 @@ module_param_named(semaphores, i915_semaphores, int, 0600);
 MODULE_PARM_DESC(semaphores,
"Use semaphores for inter-ring sync (default: -1 (use per-chip 
defaults))");
 
-unsigned int i915_enable_rc6 __read_mostly = 0;
+unsigned int i915_enable_rc6 __read_mostly = 1;
 module_param_named(i915_enable_rc6, i915_enable_rc6, int, 0600);
 MODULE_PARM_DESC(i915_enable_rc6,
"Enable power-saving render C-state 6 (default: true)");
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 981b1f1..5dd0878 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include "drmP.h"
 #include "intel_drv.h"
@@ -8746,7 +8747,7 @@ void intel_modeset_init(struct drm_device *dev)
 
 void intel_modeset_gem_init(struct drm_device *dev)
 {
-   if (IS_IRONLAKE_M(dev))
+   if (IS_IRONLAKE_M(dev) && dmar_disabled)
ironlake_enable_rc6(dev);
 
intel_setup_overlay(dev);
-- 
1.7.7.3

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


[Intel-gfx] [PATCH 2/3] drm/i915: allow semaphores on SNB if DMAR is disabled

2011-11-16 Thread Eugeni Dodonov
Semaphores cause issues when DMAR is enabled. So if we are set to per-chip
default, and we are on SNB, we can enable semaphores as long as SMAR is
disabled.

Signed-off-by: Eugeni Dodonov 
---
 drivers/gpu/drm/i915/i915_drv.c|2 ++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |4 +++-
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 355f1ab..565725c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -214,6 +214,7 @@ static const struct intel_device_info 
intel_sandybridge_d_info = {
.need_gfx_hws = 1, .has_hotplug = 1,
.has_bsd_ring = 1,
.has_blt_ring = 1,
+   .enable_semaphores = 1,
 };
 
 static const struct intel_device_info intel_sandybridge_m_info = {
@@ -222,6 +223,7 @@ static const struct intel_device_info 
intel_sandybridge_m_info = {
.has_fbc = 1,
.has_bsd_ring = 1,
.has_blt_ring = 1,
+   .enable_semaphores = 1,
 };
 
 static const struct intel_device_info intel_ivybridge_d_info = {
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 094ff4c..0510735 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -32,6 +32,7 @@
 #include "i915_drv.h"
 #include "i915_trace.h"
 #include "intel_drv.h"
+#include 
 
 struct change_domains {
uint32_t invalidate_domains;
@@ -758,7 +759,8 @@ i915_gem_execbuffer_sync_rings(struct drm_i915_gem_object 
*obj,
if (from == NULL || to == from)
return 0;
 
-   if (i915_semaphores < 0 && ENABLE_SEMAPHORES(obj->base.dev))
+   /* Only enable semaphores if DMAR is disabled */
+   if (i915_semaphores < 0 && ENABLE_SEMAPHORES(obj->base.dev) && 
dmar_disabled)
enable_semaphores = 1;
 
/* XXX gpu semaphores are implicated in various hard hangs on SNB */
-- 
1.7.7.3

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


[Intel-gfx] [PATCH 1/3] drm/i915: support for per-device semaphores settings

2011-11-16 Thread Eugeni Dodonov
This allows to enable semaphores by default on devices which support them.

By default, let's enable them on IVB only for now. When DMAR issues on SNB
will be solved, we can enable them on SNB as well.

For IVB, it should fix many hangs and issues.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42696
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=40564
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41353
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38862
Signed-off-by: Eugeni Dodonov 
---
 drivers/gpu/drm/i915/i915_drv.c|6 --
 drivers/gpu/drm/i915/i915_drv.h|3 +++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |6 +-
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index cc531bb..355f1ab 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -58,10 +58,10 @@ module_param_named(powersave, i915_powersave, int, 0600);
 MODULE_PARM_DESC(powersave,
"Enable powersavings, fbc, downclocking, etc. (default: true)");
 
-unsigned int i915_semaphores __read_mostly = 0;
+unsigned int i915_semaphores __read_mostly = -1;
 module_param_named(semaphores, i915_semaphores, int, 0600);
 MODULE_PARM_DESC(semaphores,
-   "Use semaphores for inter-ring sync (default: false)");
+   "Use semaphores for inter-ring sync (default: -1 (use per-chip 
defaults))");
 
 unsigned int i915_enable_rc6 __read_mostly = 0;
 module_param_named(i915_enable_rc6, i915_enable_rc6, int, 0600);
@@ -229,6 +229,7 @@ static const struct intel_device_info 
intel_ivybridge_d_info = {
.need_gfx_hws = 1, .has_hotplug = 1,
.has_bsd_ring = 1,
.has_blt_ring = 1,
+   .enable_semaphores = 1,
 };
 
 static const struct intel_device_info intel_ivybridge_m_info = {
@@ -237,6 +238,7 @@ static const struct intel_device_info 
intel_ivybridge_m_info = {
.has_fbc = 0,   /* FBC is not enabled on Ivybridge mobile yet */
.has_bsd_ring = 1,
.has_blt_ring = 1,
+   .enable_semaphores = 1,
 };
 
 static const struct pci_device_id pciidlist[] = {  /* aka */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 06a37f4..0e819d2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -247,6 +247,7 @@ struct intel_device_info {
u8 supports_tv:1;
u8 has_bsd_ring:1;
u8 has_blt_ring:1;
+   u8 enable_semaphores:1;
 };
 
 enum no_fbc_reason {
@@ -990,6 +991,8 @@ struct drm_i915_file_private {
 #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT)
 #define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX)
 
+#define ENABLE_SEMAPHORES(dev)(INTEL_INFO(dev)->enable_semaphores)
+
 #include "i915_trace.h"
 
 extern struct drm_ioctl_desc i915_ioctls[];
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 3693e83..094ff4c 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -753,12 +753,16 @@ i915_gem_execbuffer_sync_rings(struct drm_i915_gem_object 
*obj,
struct intel_ring_buffer *from = obj->ring;
u32 seqno;
int ret, idx;
+   int enable_semaphores = 0;
 
if (from == NULL || to == from)
return 0;
 
+   if (i915_semaphores < 0 && ENABLE_SEMAPHORES(obj->base.dev))
+   enable_semaphores = 1;
+
/* XXX gpu semaphores are implicated in various hard hangs on SNB */
-   if (INTEL_INFO(obj->base.dev)->gen < 6 || !i915_semaphores)
+   if (INTEL_INFO(obj->base.dev)->gen < 6 || !enable_semaphores)
return i915_gem_object_wait_rendering(obj);
 
idx = intel_ring_sync_index(from, to);
-- 
1.7.7.3

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


Re: [Intel-gfx] [ANNOUNCE] xf86-video-intel 2.17.0

2011-11-16 Thread Eugeni Dodonov
On Wed, Nov 16, 2011 at 21:25, Chris Wilson wrote:

> A few months have passed, and we have accumulated a surprising number of
> bug fixes. Oops! We would strongly encourage everyone to upgrade.
> -Chris
>
> Bugs fixed in this release (compared to 2.16.0)
>

...plus all the SNA-related bug fixes and changes added by the modest
number of 384 patches to the src/sna directory :).

Congrats!!

-- 
Eugeni Dodonov

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


[Intel-gfx] [ANNOUNCE] xf86-video-intel 2.17.0

2011-11-16 Thread Chris Wilson
A few months have passed, and we have accumulated a surprising number of
bug fixes. Oops! We would strongly encourage everyone to upgrade.
-Chris

Bugs fixed in this release (compared to 2.16.0)
---

 * Video clobbering composite batch state
   http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=635953

 * Incorrect reuse of surface bindings within a batch for multiple formats
   https://bugs.freedesktop.org/show_bug.cgi?id=40926

 * Nothing was rendered for text with procedural sources
   https://bugs.freedesktop.org/show_bug.cgi?id=31819

 * Handle fallbacks involving alpha maps

 * Workaround blitter hang on SandyBridge and IvyBridge
   https://bugzilla.kernel.org/show_bug.cgi?id=27892
   https://bugs.freedesktop.org/show_bug.cgi

 * Workaround pipe control issues on SandyBridge

 * Use correct maximum PS thread count on IvyBridge

 * Protect against failed pixmap allocation for XV
   https://bugs.freedesktop.org/show_bug.cgi?id=40439

git tag: 2.17.0

http://xorg.freedesktop.org/archive/individual/driver/xf86-video-intel-2.17.0.tar.bz2
MD5:  6d7b1f199dba5820f250888b136186ff  xf86-video-intel-2.17.0.tar.bz2
SHA1: 04ad9fa1f4c4e0a90f48752a709bf14700c864af  xf86-video-intel-2.17.0.tar.bz2
SHA256: 8b8450f2a2cc52ef31a83414e2f290e748a956690e11b41759d5650aaedc4387  
xf86-video-intel-2.17.0.tar.bz2

http://xorg.freedesktop.org/archive/individual/driver/xf86-video-intel-2.17.0.tar.gz
MD5:  7443f0054f9c81aad7facecbef3daef0  xf86-video-intel-2.17.0.tar.gz
SHA1: aeea2c4e8e9c25b84802c2f0dc26942ccc3590c1  xf86-video-intel-2.17.0.tar.gz
SHA256: 7da1d957b4abe6da38958f3c282d857138e7318028286dc1a1f57df5e0ff0da8  
xf86-video-intel-2.17.0.tar.gz

-- 
Chris Wilson, Intel Open Source Technology Centre


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


Re: [Intel-gfx] [alsa-devel] [PATCH 2/2] hda - delayed ELD repoll

2011-11-16 Thread Wu Fengguang
> > > Below is the dmesg representing a video mode set.
> > > 
> > > ELD writes from the graphics driver
> > > 
> > > [  424.254958] [drm:intel_write_eld], ELD on [CONNECTOR:12:HDMI-A-2], 
> > > [ENCODER:11:TMDS-11]
> > > [  424.257670] [drm:ironlake_write_eld], ELD on pipe B
> > > [  424.259833] [drm:ironlake_write_eld], Audio directed to unknown port
> > > [  424.262156] [drm:ironlake_write_eld], ELD size 13
> > > 
> > > ELD events received by audio driver (eld reads 0)
> > > 
> > > [  424.263258] HDMI hot plug event: Codec=3 Pin=6 Presence_Detect=1 
> > > ELD_Valid=0
> > 
> > That line makes sense.
> > 
> > > [  424.265877] HDMI status: Codec=3 Pin=6 Presence_Detect=1 ELD_Valid=1
> > 
> > I don't /think/ it's related to this issue, but I wonder why ELDV==1 in
> > that message; it seems that the unsolicited response contains the correct
> > data, but AC_VERB_GET_PIN_SENSE contains ELDV==1 all the time. That's odd.
> 
> It depends on timing. When audio driver receives the unsolicited event, 
> graphics driver has finished with "eld_valid = 1", hence
> AC_VERB_GET_PIN_SENSE returns ELDV=1.
> 
> It's not happening /all the time/ though. For example here is another
> dmesg showing a different timing on the same test box.

Sorry this is actually from another test box. But I did see similar
ones from the same test box.

> [  467.561516] [drm:intel_dp_mode_set], Enabling DP audio on pipe B
> [  467.567503] [drm:intel_write_eld], ELD on [CONNECTOR:26:DP-3], 
> [ENCODER:27:TMDS-27]
> [  467.574207] [drm:ironlake_write_eld], ELD on pipe B
> [  467.579346] [drm:ironlake_write_eld], Audio directed to unknown port
> [  467.584724] [drm:ironlake_write_eld], ELD: DisplayPort detected
> [  467.586540] HDMI hot plug event: Codec=3 Pin=7 Presence_Detect=1 
> ELD_Valid=0
> [  467.599608] [drm:ironlake_write_eld],
> [  467.599922] HDMI status: Codec=3 Pin=7 Presence_Detect=1 ELD_Valid=0
> [  467.605834] ELD size 9   
> [  467.610434] [drm:sandybridge_update_wm], FIFO watermarks For pipe A - 
> plane 7, cursor: 6
> [  467.612365] HDMI hot plug event: Codec=3 Pin=7 Presence_Detect=1 
> ELD_Valid=1
> [  467.620654] [drm:sandybridge_update_wm], 
> [  467.620765] HDMI status: Codec=3 Pin=7 Presence_Detect=1 ELD_Valid=1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [Mesa-dev] Mesa/Gallium overall design

2011-11-16 Thread Allen Leinwand
In a posting to intel-gfx on Mon Apr 12 10:12:12 PDT 2010, Jesse Barnes
gave several reasons why the Intel Mesa drivers team was not supporting
the Gallium drivers at that time.  Here is an excerpt from that email:

 

Moving to Gallium would be a huge effort for us.  We've invested a lot

into the current drivers, stabilizing them, adding features, and

generally supporting them.  If we moved to Gallium, much of that effort

would be thrown away as with any large rewrite, leaving users in a

situation where the driver that worked was unsupported and the one that

was supported didn't work very well (at least for quite some time).  

 

Currently, the benefits of Gallium are outweighed by this

consideration, at least in my opinion.  However, Dave has been poking

us a bit about using LLVM for a sw vertex shader implementation on 945,

which Gallium would make much easier aiui, so who knows things may

change in the future.  I still worry about delivering a high quality

and supported driver to our users though.

 

It is now a year and seven months later.  Is this still the policy of
the Intel graphics organization?

 

It appears from looking at the Classic and Gallium driver code base,
that OpenVG is only supported in the Gallium version.  Is this correct?

 

Can someone please enumerate any other feature differences between the
Classic and Gallium drivers?

 

Thank you very much in advance.

 

Allen Leinwand



Allen Leinwand
 

Teleca

, 
allen.leinw...@teleca.com
http://www.teleca.com/

Follow what's going on at Teleca's blog on 
http://www.whatsyourideaoftomorrow.blogspot.com/.

The information contained in this message is confidential and is intended for 
the addressee(s) only. If you have received this message in error please notify 
the sender immediately. The unauthorized use, disclosure, copying or alteration 
of this message is strictly prohibited.

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


Re: [Intel-gfx] [alsa-devel] [PATCH 2/2] hda - delayed ELD repoll

2011-11-16 Thread Wu Fengguang
On Wed, Nov 16, 2011 at 11:51:28PM +0800, Stephen Warren wrote:
> Wu Fengguang wrote at Tuesday, November 15, 2011 7:48 PM:
> > On Wed, Nov 16, 2011 at 02:25:00AM +0800, Stephen Warren wrote:
> > > Wu Fengguang wrote at Tuesday, November 15, 2011 7:33 AM:
> > > > The Intel HDMI chips (ironlake at least) are found to have ~250ms delay
> > > > between the ELD_Valid=1 hotplug event is send and the ELD buffer becomes
> > > > actually readable. During the time the ELD buffer is mysteriously all 0.
> > > >
> > > > Fix it by scheduling a delayed work to re-read ELD buffer after 300ms.
> > >
> > > Any idea why; is the graphics driver writing the ELD data to the audio HW
> > > after triggering the unsolicited even rather than before, or something
> > > like that?
> > 
> > Nope. The graphics driver is doing
> > 
> > eld_valid = 0
> > write to eld buffer
> > eld_valid = 1
> 
> OK, that sounds fine.
> 
> > > 250mS almost sounds like it's setting ELDV in the audio HW,
> > > then going and reading the EDID, then writing the EDID to the audio HW;
> > > perhaps the graphics driver is accidentally setting PRESENT+ELDV when it's
> > > meant to be setting just PRESENT, and later setting ELDV?
> > 
> > From the debug dmesg, I'm pretty sure that the ELDV events are
> > triggered exactly by the "eld_valid = 0" and "eld_valid = 1" register
> > writes. Since the ELD data is already prepared, there is no EDID read
> > in between.
> > 
> > Below is the dmesg representing a video mode set.
> > 
> > ELD writes from the graphics driver
> > 
> > [  424.254958] [drm:intel_write_eld], ELD on [CONNECTOR:12:HDMI-A-2], 
> > [ENCODER:11:TMDS-11]
> > [  424.257670] [drm:ironlake_write_eld], ELD on pipe B
> > [  424.259833] [drm:ironlake_write_eld], Audio directed to unknown port
> > [  424.262156] [drm:ironlake_write_eld], ELD size 13
> > 
> > ELD events received by audio driver (eld reads 0)
> > 
> > [  424.263258] HDMI hot plug event: Codec=3 Pin=6 Presence_Detect=1 
> > ELD_Valid=0
> 
> That line makes sense.
> 
> > [  424.265877] HDMI status: Codec=3 Pin=6 Presence_Detect=1 ELD_Valid=1
> 
> I don't /think/ it's related to this issue, but I wonder why ELDV==1 in
> that message; it seems that the unsolicited response contains the correct
> data, but AC_VERB_GET_PIN_SENSE contains ELDV==1 all the time. That's odd.

It depends on timing. When audio driver receives the unsolicited event, 
graphics driver has finished with "eld_valid = 1", hence
AC_VERB_GET_PIN_SENSE returns ELDV=1.

It's not happening /all the time/ though. For example here is another
dmesg showing a different timing on the same test box.

[  467.561516] [drm:intel_dp_mode_set], Enabling DP audio on pipe B
[  467.567503] [drm:intel_write_eld], ELD on [CONNECTOR:26:DP-3], 
[ENCODER:27:TMDS-27]
[  467.574207] [drm:ironlake_write_eld], ELD on pipe B
[  467.579346] [drm:ironlake_write_eld], Audio directed to unknown port
[  467.584724] [drm:ironlake_write_eld], ELD: DisplayPort detected
[  467.586540] HDMI hot plug event: Codec=3 Pin=7 Presence_Detect=1 ELD_Valid=0
[  467.599608] [drm:ironlake_write_eld],
[  467.599922] HDMI status: Codec=3 Pin=7 Presence_Detect=1 ELD_Valid=0
[  467.605834] ELD size 9   
[  467.610434] [drm:sandybridge_update_wm], FIFO watermarks For pipe A - plane 
7, cursor: 6
[  467.612365] HDMI hot plug event: Codec=3 Pin=7 Presence_Detect=1 ELD_Valid=1
[  467.620654] [drm:sandybridge_update_wm], 
[  467.620765] HDMI status: Codec=3 Pin=7 Presence_Detect=1 ELD_Valid=1

> > [  424.272415] ALSA hda_eld.c:259 HDMI: Unknown ELD version 0
> > [  424.274566] HDMI hot plug event: Codec=3 Pin=6 Presence_Detect=1 
> > ELD_Valid=1
> > [  424.277027] HDMI status: Codec=3 Pin=6 Presence_Detect=1 ELD_Valid=1
> > [  424.283157] ALSA hda_eld.c:259 HDMI: Unknown ELD version 0
> > 
> > graphics driver go on with its job
> > 
> > [  424.314331] [drm:intel_wait_for_vblank], vblank wait timed out
> > [  424.367183] [drm:intel_wait_for_vblank], vblank wait timed out
> > [  424.368960] [drm:ironlake_update_wm], FIFO watermarks For pipe A - plane 
> > 5, cursor: 6
> > [  424.370700] [drm:ironlake_update_wm], FIFO watermarks For pipe B - plane 
> > 42, cursor: 6
> > [  424.424056] [drm:intel_wait_for_vblank], vblank wait timed out
> > [  424.476906] [drm:intel_wait_for_vblank], vblank wait timed out
> > [  424.479169] [drm:ironlake_fdi_link_train], FDI_RX_IIR 0x100
> > [  424.481084] [drm:ironlake_fdi_link_train], FDI train 1 done.
> > [  424.483452] [drm:ironlake_fdi_link_train], FDI_RX_IIR 0x600
> > [  424.485444] [drm:ironlake_fdi_link_train], FDI train 2 done.
> > [  424.486765] [drm:ironlake_fdi_link_train], FDI train done
> > [  424.490820] [drm:intel_update_fbc],
> > [  424.491841] [drm:drm_crtc_helper_set_config], Setting connector DPMS 
> > state to on
> > [  424.494449] [drm:drm_crtc_helper_set_config],
> > [CONNECTOR:12:HDMI-A-2] set DPMS on
> > [  424.505904] [drm:i

Re: [Intel-gfx] [PATCH 1/2] drm/i915: re-enable semaphores by default

2011-11-16 Thread Keith Packard
On Wed, 16 Nov 2011 21:56:37 +0100 (CET), "Nicolas Kalkhof"  
wrote:

> Not quite. On my i7 2620M (Lenovo T420) the gpu frequently hangs when
> decoding videos (vaapi) and running OpenGL apps/games at the same
> time. Disabling iommu makes my system relatively stable even with rc6
> enabled. I haven't played with the semaphores however.

So, if we disabled rc6 when iommu is enabled (dmar_disable = 0), we
should be in good shape then.

-- 
keith.pack...@intel.com


pgpeKemZMe720.pgp
Description: PGP signature
___
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: re-enable semaphores by default

2011-11-16 Thread Keith Packard
On Wed, 16 Nov 2011 21:18:13 +0100, Daniel Vetter  
wrote:

> Last time I've played around with all the combinations, only
> intel_iommu=off was good enough to prevent the hang.
> intel_iommu=igd_off still hangs (which is what the current value
> exported by the dmar code dopends on, iirc). If I remember things
> correctly, intel_iommu=off also reliably works around issues for all
> reporters (both semaphores and rc6).

So, the dmar_disabled flag would suffice? Should be trivial to export
that and check in our driver.

-- 
keith.pack...@intel.com


pgpzri5pUuZyX.pgp
Description: PGP signature
___
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: re-enable semaphores by default

2011-11-16 Thread Daniel Vetter
On Wed, Nov 16, 2011 at 21:56, Nicolas Kalkhof  wrote:
> Not quite. On my i7 2620M (Lenovo T420) the gpu frequently hangs when 
> decoding videos (vaapi) and running OpenGL apps/games at the same time. 
> Disabling iommu makes my system relatively stable even with rc6 enabled. I 
> haven't played with the semaphores however.

Can you please try my ppgtt branch from my fdo repo at
http://cgit.freedesktop.org/~danvet/drm/ ? Note thought that it'll
only work on snb/ivb and that suspend/resume is broken. Also, it won't
properly re-enable ppgtt when resetting the gpu after a hang.
But testing feedback highly appreciated.

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


Re: [Intel-gfx] [PATCH 1/2] drm/i915: re-enable semaphores by default

2011-11-16 Thread Nicolas Kalkhof
Not quite. On my i7 2620M (Lenovo T420) the gpu frequently hangs when decoding 
videos (vaapi) and running OpenGL apps/games at the same time. Disabling iommu 
makes my system relatively stable even with rc6 enabled. I haven't played with 
the semaphores however.

-- Nic


-Ursprüngliche Nachricht-
Von: "Andrew Lutomirski" 
Gesendet: Nov 16, 2011 6:24:54 PM
An: "Keith Packard" 
Betreff: Re: [Intel-gfx] [PATCH 1/2] drm/i915: re-enable semaphores by default

>On Wed, Nov 16, 2011 at 9:16 AM, Keith Packard  wrote:
>> On Wed, 16 Nov 2011 16:49:40 +0100, Daniel Vetter  
>> wrote:
>>
>>> So we need to check whether DMAR is enabled (on the
>>> entire system, i.e. the variable exported for the ilk workaround is
>>> not good enough)
>>
>> Can you figure out what *would* be sufficient? Getting semaphores turned
>> on for the 99% who aren't enabling VT-d would be a fairly nice
>> performance improvement.
>
>AFAICT my snb laptop has always been stable with semaphores and VT-d
>enabled. Is this problem possibly restricted to just desktop
>machines? I'm happy to test, since my box that can reproduce the hang
>instantly is still around.
>
>Also, rc6-by-default would be very nice. It decreases the total power
>consumption on my laptop from over 13W to around 8W. That's huge.
>
>--Andy
>___
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/intel-gfx


___
SMS schreiben mit WEB.DE FreeMail - einfach, schnell und
kostenguenstig. Jetzt gleich testen! http://f.web.de/?mc=021192
___
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: re-enable rc6 by default

2011-11-16 Thread Daniel Vetter
On Wed, Nov 16, 2011 at 16:59, Andrew Lutomirski  wrote:
>
> On Nov 16, 2011 7:54 AM, "Daniel Vetter"  wrote:
>>
>> On Mon, Nov 14, 2011 at 21:39, Eugeni Dodonov 
>> wrote:
>> > Most of the rc6-related hangs and major issues were addressed for the
>> > past
>> > months.
>> >
>> > Let's re-enable it by default to provide a more wider testing, and catch
>> > the remaining problems.
>> >
>> > According to tests, enablement of rc6 results in up to +50% improvements
>> > in power usage and battery life, so it certainly would be a nice feature
>> > to have enabled by default.
>> >
>> > Also, most of the issues related to rc6 seem to came from VTd, so if you
>> > are experiencing any problems with it, try disabling VTd in bios or
>> > using
>> > intel_iommu=off kernel parameter to investigate whether it solves the
>> > issue.
>> >
>> > Acked-by: Keith Packard 
>> > CC: Daniel Vetter 
>> > CC: Jesse Barnes 
>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38567
>> > Signed-off-by: Eugeni Dodonov 
>>
>> Iirc the same applies to rc6 as to semaphores. We have bug reports
>> that it causes hard-hangs in combination with DMAR. I haven't yet
>> gotten around to poking the relevant reporters whether ppgtt changes
>> anything, because internet access here at the Intel Poland site sucks.
>> So again I think we need to disable this on snb when DMAR is on.
>
> I can reproduce the semaphore hang easily.  Where are the ppgtt
> patches/settings to play with?
>
> FWIW, rc6 is a *huge* win on my SNB laptop, and the laptop has always been
> rock-solid with any set of options.  Maybe this is only an issue on desktop
> parts, and the 5W difference is a bigger deal on laptops anyway.

Grab the ppgtt branch from my fdo repo:

http://cgit.freedesktop.org/~danvet/drm/

Note that this branch will only work on snb/ivb and that resume is
broken. Also, after the first gpu reset ppgtt will get disabled and
hence your machine might die rather quickly. So perhaps boot with
i915.reset=0 to prevent that.

Testing feedback highly welcome.

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


Re: [Intel-gfx] [PATCH 1/2] drm/i915: re-enable semaphores by default

2011-11-16 Thread Daniel Vetter
On Wed, Nov 16, 2011 at 18:16, Keith Packard  wrote:
> On Wed, 16 Nov 2011 16:49:40 +0100, Daniel Vetter  
> wrote:
>
>> So we need to check whether DMAR is enabled (on the
>> entire system, i.e. the variable exported for the ilk workaround is
>> not good enough)
>
> Can you figure out what *would* be sufficient? Getting semaphores turned
> on for the 99% who aren't enabling VT-d would be a fairly nice
> performance improvement.

Last time I've played around with all the combinations, only
intel_iommu=off was good enough to prevent the hang.
intel_iommu=igd_off still hangs (which is what the current value
exported by the dmar code dopends on, iirc). If I remember things
correctly, intel_iommu=off also reliably works around issues for all
reporters (both semaphores and rc6).

And for reproducing it, at least here the key ingredient seems to be a
kde4 desktop. Spare the jokes ;-)
-Daniel
-- 
Daniel Vetter
daniel.vet...@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/2] drm/i915: re-enable semaphores by default

2011-11-16 Thread Jesse Barnes
On Wed, 16 Nov 2011 17:05:37 -0200
Eugeni Dodonov  wrote:

> On Wed, Nov 16, 2011 at 15:24, Andrew Lutomirski  wrote:
> 
> > AFAICT my snb laptop has always been stable with semaphores and VT-d
> > enabled.  Is this problem possibly restricted to just desktop
> > machines?  I'm happy to test, since my box that can reproduce the hang
> > instantly is still around.
> >
> 
> I cannot reproduce those issues on any mobile SNB, but I am out of desktop
> ones. So perhaps it is a clue.
> 
> Jesse, Keith, Daniel, what if we add something like:
> 
> ...
> .has_semaphores=1, .has_rc6=1
> ...
> 
> to i915_drv.c intel_sandybridge_m_info, and take them into account when
> using default values for those settings?

Something like that is fine with me, or just doing it in modeset_init
for certain chipsets.

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


Re: [Intel-gfx] [PATCH 1/2] drm/i915: re-enable semaphores by default

2011-11-16 Thread Eugeni Dodonov
On Wed, Nov 16, 2011 at 15:24, Andrew Lutomirski  wrote:

> AFAICT my snb laptop has always been stable with semaphores and VT-d
> enabled.  Is this problem possibly restricted to just desktop
> machines?  I'm happy to test, since my box that can reproduce the hang
> instantly is still around.
>

I cannot reproduce those issues on any mobile SNB, but I am out of desktop
ones. So perhaps it is a clue.

Jesse, Keith, Daniel, what if we add something like:

...
.has_semaphores=1, .has_rc6=1
...

to i915_drv.c intel_sandybridge_m_info, and take them into account when
using default values for those settings?

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


Re: [Intel-gfx] [RFC] Reduce idle vblank wakeups

2011-11-16 Thread Matthew Garrett
On Wed, Nov 16, 2011 at 07:27:51PM +0100, Mario Kleiner wrote:

> It's not broken hardware, but fast ping-ponging it on and off can
> make the vblank counter and vblank timestamps unreliable for apps
> that need high timing precision, especially for the ones that use
> the OML_sync_control extensions for precise swap scheduling. My
> target application is vision science
>  neuroscience, where (sub-)milliseconds often matter for visual
> stimulation.

I'll admit that I'm struggling to understand the issue here. If the 
vblank counter is incremented at the time of vblank (which isn't the 
case for radeon, it seems, but as far as I can tell is the case for 
Intel) then how does ping-ponging the IRQ matter? 
vblank_disable_and_save() appears to handle this case.

> I think making the vblank off delay driver specific via these
> patches is a good idea. Lowering the timeout to something like a few
> refresh cycles, maybe somewhere between 50 msecs and 100 msecs would
> be also fine by me. I still would like to keep some drm config
> option to disable or override the vblank off delay by users.

Does the timeout serve any purpose other than letting software 
effectively prevent vblanks from being disabled?

> The intel and radeon kms drivers implement everything that's needed
> to make it mostly work. Except for a small race between the cpu and
> gpu in the vblank_disable_and_save() function  electrons.com/source/drivers/gpu/drm/drm_irq.c#L101> and
> drm_update_vblank_count(). It can cause an off-by-one error when
> reinitializing the drm vblank counter from the gpu's hardware
> counter if the enable/disable function is called at the wrong moment
> while the gpu's scanout is inside the vblank interval, see comments
> in the code. I have some sketchy idea for a patch that could detect
> when the race happens and retry hw counter queries to fix this.
> Without that patch, there's some chance between 0% and 4% of being
> off-by-one.

For Radeon, I'd have thought you could handle this by scheduling an irq 
for the beginning of scanout (avivo has a register for that) and 
delaying the vblank disable until you hit it?

> On current nouveau kms, disabling vblank irqs guarantees you wrong
> vblank counts and wrong vblank timestamps after turning them on
> again, because the kms driver doesn't implement the hook for
> hardware vblank counter query correctly. The drm vblank counter
> misses all counts during the vblank irq off period. Other timing
> related hooks are missing as well. I have a couple of patches queued
> up and some more to come for the ddx and kms driver to mostly fix
> this. NVidia gpu's only have hardware vblank counters for NV-50 and
> later, fixing this for earlier gpu's would require some emulation of
> a hw vblank counter inside the kms driver.

I've no problem with all of this work being case by case.

> Apps that rely on the vblank counter being totally reliable over
> long periods of time currently would be in a bad situation with a
> lowered vblank off delay, but that's probably generally not a good
> assumption. Toolkits like mine, which are more paranoid, currently
> can work fine as long as the off delay is at least a few video
> refresh cycles. I do the following for scheduling a reliably timed
> swap:
> 
> 1. Query current vblank counter current_msc and vblank timestamp
> current_ust.
> 2. Calculate a target vblank count target_msc, based on current_msc,
> current_ust and some target time from usercode.
> 3. Schedule bufferswap for target_msc.
> 
> As long as the vblank off delay is long enough so that vblanks don't
> get turned off between 1. and 3, everything is fine, otherwise bad
> things will happen.
> Keeping a way to override the default off delay would be good to
> allow poor scientists to work around potentially broken drivers or
> gpu's in the future. @Matthew: I'm appealing here to your ex-
> Drosophila biologist heritage ;-)

If vblanks are disabled and then re-enabled between 1 and 3, what's the 
negative outcome?

-- 
Matthew Garrett | mj...@srcf.ucam.org
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [RFC] Reduce idle vblank wakeups

2011-11-16 Thread Andrew Lutomirski
2011/11/16 Matthew Garrett :
> On Wed, Nov 16, 2011 at 06:03:15PM +0100, Michel Dänzer wrote:
>
>> I thought the main reason for the delay wasn't broken hardware but to
>> avoid constantly ping-ponging the vblank IRQ between on and off with
>> apps which regularly neeed the vblank counter value, as that could make
>> the counter unreliable. Maybe I'm misremembering though.
>
> If turning it on and off results in the counter value being wrong then
> surely that's a hardware problem? I've tested that turning it on and off
> between every IRQ still gives valid counter values on sandybridge.

This, and the thread that contains it, might be interesting:

http://permalink.gmane.org/gmane.comp.video.dri.devel/53201

In summary: there was some resistance to doing this in January, but I
couldn't find any legitimate reason.

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


Re: [Intel-gfx] [RFC] Reduce idle vblank wakeups

2011-11-16 Thread Matthew Garrett
On Wed, Nov 16, 2011 at 06:03:15PM +0100, Michel Dänzer wrote:

> I thought the main reason for the delay wasn't broken hardware but to
> avoid constantly ping-ponging the vblank IRQ between on and off with
> apps which regularly neeed the vblank counter value, as that could make
> the counter unreliable. Maybe I'm misremembering though.

If turning it on and off results in the counter value being wrong then 
surely that's a hardware problem? I've tested that turning it on and off 
between every IRQ still gives valid counter values on sandybridge.

-- 
Matthew Garrett | mj...@srcf.ucam.org
___
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: re-enable semaphores by default

2011-11-16 Thread Andrew Lutomirski
On Wed, Nov 16, 2011 at 9:16 AM, Keith Packard  wrote:
> On Wed, 16 Nov 2011 16:49:40 +0100, Daniel Vetter  
> wrote:
>
>> So we need to check whether DMAR is enabled (on the
>> entire system, i.e. the variable exported for the ilk workaround is
>> not good enough)
>
> Can you figure out what *would* be sufficient? Getting semaphores turned
> on for the 99% who aren't enabling VT-d would be a fairly nice
> performance improvement.

AFAICT my snb laptop has always been stable with semaphores and VT-d
enabled.  Is this problem possibly restricted to just desktop
machines?  I'm happy to test, since my box that can reproduce the hang
instantly is still around.

Also, rc6-by-default would be very nice.  It decreases the total power
consumption on my laptop from over 13W to around 8W.  That's huge.

--Andy
___
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: re-enable semaphores by default

2011-11-16 Thread Keith Packard
On Wed, 16 Nov 2011 16:49:40 +0100, Daniel Vetter  
wrote:

> So we need to check whether DMAR is enabled (on the
> entire system, i.e. the variable exported for the ilk workaround is
> not good enough)

Can you figure out what *would* be sufficient? Getting semaphores turned
on for the 99% who aren't enabling VT-d would be a fairly nice
performance improvement.

-- 
keith.pack...@intel.com


pgp2bBgO64y9R.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Fix invalid backpanel values for GEN3 or older chips

2011-11-16 Thread Takashi Iwai
While refactoring of backlight control code in commit [a95735569:
drm/i915: Refactor panel backlight controls], the handling of the bit
0 of duty-cycle was gone except for pineview.  This resulted in invalid
register values for old chips like 915GM.  When the bit 0 is set, the
backlight is turned off suddenly.

This patch changes the bit-0 check by replacing with the condition of
gen < 4 (pineview is included in this condition, too).

Reported-and-tested-by: Daniel Mack 
Cc: 
Signed-off-by: Takashi Iwai 
---
 drivers/gpu/drm/i915/intel_panel.c |8 +++-
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_panel.c 
b/drivers/gpu/drm/i915/intel_panel.c
index 499d4c0..737d00f 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -178,12 +178,10 @@ u32 intel_panel_get_max_backlight(struct drm_device *dev)
if (HAS_PCH_SPLIT(dev)) {
max >>= 16;
} else {
-   if (IS_PINEVIEW(dev)) {
+   if (INTEL_INFO(dev)->gen < 4) {
max >>= 17;
} else {
max >>= 16;
-   if (INTEL_INFO(dev)->gen < 4)
-   max &= ~1;
}
 
if (is_backlight_combination_mode(dev))
@@ -203,7 +201,7 @@ u32 intel_panel_get_backlight(struct drm_device *dev)
val = I915_READ(BLC_PWM_CPU_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
} else {
val = I915_READ(BLC_PWM_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
-   if (IS_PINEVIEW(dev))
+   if (INTEL_INFO(dev)->gen < 4)
val >>= 1;
 
if (is_backlight_combination_mode(dev)) {
@@ -246,7 +244,7 @@ static void intel_panel_actually_set_backlight(struct 
drm_device *dev, u32 level
}
 
tmp = I915_READ(BLC_PWM_CTL);
-   if (IS_PINEVIEW(dev)) {
+   if (INTEL_INFO(dev)->gen < 4) {
tmp &= ~(BACKLIGHT_DUTY_CYCLE_MASK - 1);
level <<= 1;
} else
-- 
1.7.7
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/3] drm/i915: add destination color key support

2011-11-16 Thread Jesse Barnes
On Wed, 16 Nov 2011 17:10:53 +0100
Daniel Vetter  wrote:

> On Mon, Nov 14, 2011 at 21:22, Jesse Barnes  wrote:
> > Add new ioctls for getting and setting the current destination color
> > key.  This allows for simple overlay display control by matching a color
> > key value in the primary plane before blending the overlay on top.
> >
> > Signed-off-by: Jesse Barnes 
> 
> Just a few comments because reviewing patches here is a royal pain:
> - Is taking dev->struct_mutex really required around the register access?

Oh did I leave them in place for these paths?  I'll drop them too.

> - You unconditionally enable the color key. Do we anticipate any other
> uses (like no color key or dst color key) or is this just the
> make-Xv-work ioctl?

I was thinking of that...  W/o a src or dest key, the overlay will
always be on top.  That's probably reasonable for simple cases, so
having a "key present" flag makes sense.

Which means setting a 0 dest key would mean "disable dest key" and go
back to the default behavior.  So the interface would stay the same
even if we added other key types in the future.

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


Re: [Intel-gfx] [PATCH 3/3] drm/i915: track sprite coverage and disable primary plane if possible

2011-11-16 Thread Jesse Barnes
On Wed, 16 Nov 2011 17:05:50 +0100
Daniel Vetter  wrote:

> On Mon, Nov 14, 2011 at 21:22, Jesse Barnes  wrote:
> > To save power when the sprite is full screen, we can disable the primary
> > plane on the same pipe.  Track the sprite status and enable/disable the
> > primary opportunistically.
> >
> > Signed-off-by: Jesse Barnes
> 
> Imo the indirection is overkill because afaics there's no difference.
> And in my experience this will only get in the way when later hw
> _really_ needs some abstraction (*cough* intel_ringbuffer.c *cough*.
> Can we just kill it?

Yeah I can make ivb and snb share at least (and probably ilk), but I
*do* have code coming that needs the separation.  (At least I'm pretty
sure; I'll remove the abstraction if it turns out I don't need it.)

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


Re: [Intel-gfx] [alsa-devel] [PATCH 2/2] hda - delayed ELD repoll

2011-11-16 Thread Takashi Iwai
At Wed, 16 Nov 2011 08:12:04 -0800,
Stephen Warren wrote:
> 
> Takashi Iwai wrote at Wednesday, November 16, 2011 8:58 AM:
> > At Wed, 16 Nov 2011 07:51:28 -0800, Stephen Warren wrote:
> ...
> > > > [  424.263258] HDMI hot plug event: Codec=3 Pin=6 Presence_Detect=1 
> > > > ELD_Valid=0
> > >
> > > That line makes sense.
> > >
> > > > [  424.265877] HDMI status: Codec=3 Pin=6 Presence_Detect=1 ELD_Valid=1
> > >
> > > I don't /think/ it's related to this issue, but I wonder why ELDV==1 in
> > > that message; it seems that the unsolicited response contains the correct
> > > data, but AC_VERB_GET_PIN_SENSE contains ELDV==1 all the time. That's odd.
> > 
> > Note that this bit isn't from GET_PIN_SENSE verb but the bit in the
> > unsol event argument.
> 
> The first message above prints data from the unsolicited event. The second
> message above is derived from the GET_PIN_SENSE verb.

Ah, right.  Too similar lines...


thanks,

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


Re: [Intel-gfx] [PATCH 2/3] drm/i915: add destination color key support

2011-11-16 Thread Daniel Vetter
On Mon, Nov 14, 2011 at 21:22, Jesse Barnes  wrote:
> Add new ioctls for getting and setting the current destination color
> key.  This allows for simple overlay display control by matching a color
> key value in the primary plane before blending the overlay on top.
>
> Signed-off-by: Jesse Barnes 

Just a few comments because reviewing patches here is a royal pain:
- Is taking dev->struct_mutex really required around the register access?
- You unconditionally enable the color key. Do we anticipate any other
uses (like no color key or dst color key) or is this just the
make-Xv-work ioctl?
- Same for the mask 

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


Re: [Intel-gfx] [PATCH 3/3] drm/i915: track sprite coverage and disable primary plane if possible

2011-11-16 Thread Daniel Vetter
On Mon, Nov 14, 2011 at 21:22, Jesse Barnes  wrote:
> To save power when the sprite is full screen, we can disable the primary
> plane on the same pipe.  Track the sprite status and enable/disable the
> primary opportunistically.
>
> Signed-off-by: Jesse Barnes

Imo the indirection is overkill because afaics there's no difference.
And in my experience this will only get in the way when later hw
_really_ needs some abstraction (*cough* intel_ringbuffer.c *cough*.
Can we just kill it?
-Daniel
-- 
Daniel Vetter
daniel.vet...@ffwll.ch - +41 (0) 79 364 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/3] drm/i915: add SNB and IVB video sprite support v2

2011-11-16 Thread Daniel Vetter
On Mon, Nov 14, 2011 at 21:22, Jesse Barnes  wrote:
> The video sprites support various video surface formats natively and can
> handle scaling as well.  So add support for them using the new DRM core
> sprite support functions.
>
> v2: use drm specific fourcc header and defines
> v3: address Daniel's comments:
>  - don't take struct mutex around register access (only needed for
>    regs in the GT power well)
>  - don't hold struct mutex across vblank waits
>  - fix up update_plane API (pass obj instead of GTT offset)
>  - add interlaced defines for sprite regs
>  - drop unnecessary 'reg' variables
>  - comment double buffered reg flushing
>  Also fix w/h confusion when writing the scaling reg.
>
> For this version, I tested DPMS since it came up in the last review;
> DPMS off/on works ok when a video player is working under X, but for
> power saving we'll probably want to do something smarter.  I'll leave
> that for a separate patch on top.  Likewise with the refcounting/fb
> layer handling, which are really separate cleanups.
>
> Signed-off-by: Jesse Barnes 
I haven't rechecked with Bspec (I'm not that insane ;-) but with my
comments addressed, this looks good, and fixing dpms handling and
framebuffer ugliness is something for another patch series.
Reviewed-by: Daniel Vetter 
-- 
Daniel Vetter
daniel.vet...@ffwll.ch - +41 (0) 79 364 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] [alsa-devel] [PATCH 2/2] hda - delayed ELD repoll

2011-11-16 Thread Takashi Iwai
At Wed, 16 Nov 2011 07:51:28 -0800,
Stephen Warren wrote:
> 
> > > 250mS almost sounds like it's setting ELDV in the audio HW,
> > > then going and reading the EDID, then writing the EDID to the audio HW;
> > > perhaps the graphics driver is accidentally setting PRESENT+ELDV when it's
> > > meant to be setting just PRESENT, and later setting ELDV?
> > 
> > From the debug dmesg, I'm pretty sure that the ELDV events are
> > triggered exactly by the "eld_valid = 0" and "eld_valid = 1" register
> > writes. Since the ELD data is already prepared, there is no EDID read
> > in between.
> > 
> > Below is the dmesg representing a video mode set.
> > 
> > ELD writes from the graphics driver
> > 
> > [  424.254958] [drm:intel_write_eld], ELD on [CONNECTOR:12:HDMI-A-2], 
> > [ENCODER:11:TMDS-11]
> > [  424.257670] [drm:ironlake_write_eld], ELD on pipe B
> > [  424.259833] [drm:ironlake_write_eld], Audio directed to unknown port
> > [  424.262156] [drm:ironlake_write_eld], ELD size 13
> > 
> > ELD events received by audio driver (eld reads 0)
> > 
> > [  424.263258] HDMI hot plug event: Codec=3 Pin=6 Presence_Detect=1 
> > ELD_Valid=0
> 
> That line makes sense.
> 
> > [  424.265877] HDMI status: Codec=3 Pin=6 Presence_Detect=1 ELD_Valid=1
> 
> I don't /think/ it's related to this issue, but I wonder why ELDV==1 in
> that message; it seems that the unsolicited response contains the correct
> data, but AC_VERB_GET_PIN_SENSE contains ELDV==1 all the time. That's odd.

Note that this bit isn't from GET_PIN_SENSE verb but the bit in the
unsol event argument.


Takashi
___
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: re-enable rc6 by default

2011-11-16 Thread Daniel Vetter
On Mon, Nov 14, 2011 at 21:39, Eugeni Dodonov  wrote:
> Most of the rc6-related hangs and major issues were addressed for the past
> months.
>
> Let's re-enable it by default to provide a more wider testing, and catch
> the remaining problems.
>
> According to tests, enablement of rc6 results in up to +50% improvements
> in power usage and battery life, so it certainly would be a nice feature
> to have enabled by default.
>
> Also, most of the issues related to rc6 seem to came from VTd, so if you
> are experiencing any problems with it, try disabling VTd in bios or using
> intel_iommu=off kernel parameter to investigate whether it solves the
> issue.
>
> Acked-by: Keith Packard 
> CC: Daniel Vetter 
> CC: Jesse Barnes 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38567
> Signed-off-by: Eugeni Dodonov 

Iirc the same applies to rc6 as to semaphores. We have bug reports
that it causes hard-hangs in combination with DMAR. I haven't yet
gotten around to poking the relevant reporters whether ppgtt changes
anything, because internet access here at the Intel Poland site sucks.
So again I think we need to disable this on snb when DMAR is on.
-Daniel
-- 
Daniel Vetter
daniel.vet...@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/2] drm/i915: re-enable semaphores by default

2011-11-16 Thread Daniel Vetter
On Mon, Nov 14, 2011 at 21:39, Eugeni Dodonov  wrote:
> Semaphores seem to fix most of the hangs on SNB and IVB, and do not cause
> any known regressions as of now.
>
> Let's re-enable them by default to provide a wider testing and coverage.
>
> Acked-by: Keith Packard 
> CC: Jesse Barnes 
> CC: Daniel Vetter 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42696
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=40564
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41353
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38862
> Signed-off-by: Eugeni Dodonov 

Semaphores are still broken on my snb in combination with DMAR. ppgtt
seems to slightly change the failure mode and probably helps in
tracking this down, but without ppgtt it's a hard hang a few seconds
after login. So we need to check whether DMAR is enabled (on the
entire system, i.e. the variable exported for the ilk workaround is
not good enough) on snb and then disable it by default. So in this
current form:
Nacked-by: Daniel Vetter  (still causes easily
reproducible hard-hangs)

I don't have anything against enabling this by default on ivb.
-- 
Daniel Vetter
daniel.vet...@ffwll.ch - +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] How to disable the internal graphics device of Q45 for lower power consumption

2011-11-16 Thread joseph wang
Hi all,
I wonder if there is a way to disable or power off the built-in graphics
device that is integrated in chipset if it is not used for most of the
time, for instance, on servers. What I have tried is, experiment on Q45
chipset, set bit 3 and 4 of register 'deven' at offset 54-57h in the PCI
configuration space belonging to device 0 function 0. The datasheet of Q45
tells setting these bits can disable the internal graphics(deveice 2
function 0 and 1). I tried and it turned out that if the VGA cable is
connected, setting the bits leads up to shut-off of signal and the power
meter shows less dissipation. That is great. But if I unplug the VGA cable,
I can also see the same drop in degree as much as which if I disable the
device. That means if cable is unconnected, disable the device cannot
further reduce the power consumption. So I am guessing the graphics device
is not fully powered off by this method. The questions goes, is there any
way which can fully power off the device or at least put it into much much
lower power state. If that is possible I can do it on our online servers to
save energy. The sever is Atom based platform with GMA 3150 graphics
integrated.
Thanks,
Joseph**
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [RFC] Reduce idle vblank wakeups

2011-11-16 Thread Adam Jackson
On Wed, 2011-11-16 at 09:20 -0500, Matthew Garrett wrote:
> The drm core currently waits 5 seconds from userspace dropping a request
> for vblanks to vblanks actually being disabled. This appears to be a
> workaround for broken hardware, but results in a mostly idle desktop
> generating a huge number of wakeups that are entirely unnecessary but which
> consume measurable amounts of power. This patchset makes the vblank timeout
> per-device rather than global, making it easy for drivers to override the
> behaviour without compromising other graphics hardware in the system. It
> then removes the delay on Intel hardware. I've tested this successfully on
> Sandybridge without any evidence of spurious or missing irqs, but I don't
> know how well it works on older hardware. Feedback not only welcome, but
> positively desired.

Looks good to me.  I'll test this on some other intel kit I've got
handy.  For the series:

Reviewed-by: Adam Jackson 

- ajax


signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [RFC 3/3] i915: Drop vblank_offdelay

2011-11-16 Thread Matthew Garrett
Sandybridge, at least, seems to manage without any vblank offdelay.
Dropping this reduces the number of wakeups on an otherwise idle system
dramatically.

Signed-off-by: Matthew Garrett 
---
 drivers/gpu/drm/i915/i915_dma.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index a9533c5..46e7172 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1917,6 +1917,9 @@ int i915_driver_load(struct drm_device *dev, unsigned 
long flags)
goto free_priv;
}
 
+   /* vblank is reliable */
+   dev->vblank_offdelay = 0;
+
/* overlay on gen2 is broken and can't address above 1G */
if (IS_GEN2(dev))
dma_set_coherent_mask(&dev->pdev->dev, DMA_BIT_MASK(30));
-- 
1.7.7.1

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


[Intel-gfx] [RFC 2/3] drm: Handle the vblank_offdelay=0 case properly

2011-11-16 Thread Matthew Garrett
Right now if vblank_offdelay is 0, vblanks won't be disabled after the
last user. Fix that case up.

Signed-off-by: Matthew Garrett 
---
 drivers/gpu/drm/drm_irq.c |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 8bcb6a4..94f9579 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -935,8 +935,7 @@ void drm_vblank_put(struct drm_device *dev, int crtc)
BUG_ON(atomic_read(&dev->vblank_refcount[crtc]) == 0);
 
/* Last user schedules interrupt disable */
-   if (atomic_dec_and_test(&dev->vblank_refcount[crtc]) &&
-   (dev->vblank_offdelay > 0))
+   if (atomic_dec_and_test(&dev->vblank_refcount[crtc]))
mod_timer(&dev->vblank_disable_timer,
  jiffies + ((dev->vblank_offdelay * DRM_HZ)/1000));
 }
-- 
1.7.7.1

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


[Intel-gfx] [RFC 1/3] drm: Make drm_vblank_offdelay per-device

2011-11-16 Thread Matthew Garrett
drm_vblank_offdelay is currently a system global, despite the optimal
value being hardware-specific. Move it to the drm_device structure.

Signed-off-by: Matthew Garrett 
---
 drivers/gpu/drm/drm_irq.c  |6 +++---
 drivers/gpu/drm/drm_stub.c |8 +---
 include/drm/drmP.h |2 +-
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index cb3794a..8bcb6a4 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -928,7 +928,7 @@ EXPORT_SYMBOL(drm_vblank_get);
  * @crtc: which counter to give up
  *
  * Release ownership of a given vblank counter, turning off interrupts
- * if possible. Disable interrupts after drm_vblank_offdelay milliseconds.
+ * if possible. Disable interrupts after vblank_offdelay milliseconds.
  */
 void drm_vblank_put(struct drm_device *dev, int crtc)
 {
@@ -936,9 +936,9 @@ void drm_vblank_put(struct drm_device *dev, int crtc)
 
/* Last user schedules interrupt disable */
if (atomic_dec_and_test(&dev->vblank_refcount[crtc]) &&
-   (drm_vblank_offdelay > 0))
+   (dev->vblank_offdelay > 0))
mod_timer(&dev->vblank_disable_timer,
- jiffies + ((drm_vblank_offdelay * DRM_HZ)/1000));
+ jiffies + ((dev->vblank_offdelay * DRM_HZ)/1000));
 }
 EXPORT_SYMBOL(drm_vblank_put);
 
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 6d7b083..189a077 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -40,8 +40,8 @@
 unsigned int drm_debug = 0;/* 1 to enable debug output */
 EXPORT_SYMBOL(drm_debug);
 
-unsigned int drm_vblank_offdelay = 5000;/* Default to 5000 msecs. */
-EXPORT_SYMBOL(drm_vblank_offdelay);
+unsigned int drm_default_vblank_offdelay = 5000;   /* Default to 5000 msecs. */
+EXPORT_SYMBOL(drm_default_vblank_offdelay);
 
 unsigned int drm_timestamp_precision = 20;  /* Default to 20 usecs. */
 EXPORT_SYMBOL(drm_timestamp_precision);
@@ -54,7 +54,7 @@ MODULE_PARM_DESC(vblankoffdelay, "Delay until vblank irq 
auto-disable [msecs]");
 MODULE_PARM_DESC(timestamp_precision_usec, "Max. error on timestamps [usecs]");
 
 module_param_named(debug, drm_debug, int, 0600);
-module_param_named(vblankoffdelay, drm_vblank_offdelay, int, 0600);
+module_param_named(vblankoffdelay, drm_default_vblank_offdelay, int, 0600);
 module_param_named(timestamp_precision_usec, drm_timestamp_precision, int, 
0600);
 
 struct idr drm_minors_idr;
@@ -290,6 +290,8 @@ int drm_fill_in_dev(struct drm_device *dev,
 
dev->driver = driver;
 
+   dev->vblank_offdelay = drm_default_vblank_offdelay;
+
if (dev->driver->bus->agp_init) {
retcode = dev->driver->bus->agp_init(dev);
if (retcode)
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index cf39949..81e6bbb 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1168,6 +1168,7 @@ struct drm_device {
struct idr object_name_idr;
/*@} */
int switch_power_state;
+   int vblank_offdelay;
 };
 
 #define DRM_SWITCH_POWER_ON 0
@@ -1463,7 +1464,6 @@ extern void drm_put_dev(struct drm_device *dev);
 extern int drm_put_minor(struct drm_minor **minor);
 extern unsigned int drm_debug;
 
-extern unsigned int drm_vblank_offdelay;
 extern unsigned int drm_timestamp_precision;
 
 extern struct class *drm_class;
-- 
1.7.7.1

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


[Intel-gfx] [RFC] Reduce idle vblank wakeups

2011-11-16 Thread Matthew Garrett
The drm core currently waits 5 seconds from userspace dropping a request
for vblanks to vblanks actually being disabled. This appears to be a
workaround for broken hardware, but results in a mostly idle desktop
generating a huge number of wakeups that are entirely unnecessary but which
consume measurable amounts of power. This patchset makes the vblank timeout
per-device rather than global, making it easy for drivers to override the
behaviour without compromising other graphics hardware in the system. It
then removes the delay on Intel hardware. I've tested this successfully on
Sandybridge without any evidence of spurious or missing irqs, but I don't
know how well it works on older hardware. Feedback not only welcome, but
positively desired.

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


[Intel-gfx] [PATCH 3/3 v2] drm/i915: hot removal notification to HDMI audio driver

2011-11-16 Thread Wu Fengguang
On monitor hot removal:

1) clear SDVO_AUDIO_ENABLE or DP_AUDIO_OUTPUT_ENABLE
2) clear ELD Valid bit

So that the audio driver will receive hot plug events and take action to
refresh its device state and ELD contents.

cc: Wang Zhenyu 
Signed-off-by: Wu Fengguang 
---
 drivers/gpu/drm/drm_crtc_helper.c |4 
 drivers/gpu/drm/i915/intel_dp.c   |   17 +
 drivers/gpu/drm/i915/intel_hdmi.c |   17 +
 include/drm/drm_crtc.h|1 +
 4 files changed, 39 insertions(+)

--- linux.orig/drivers/gpu/drm/i915/intel_dp.c  2011-11-16 21:36:58.0 
+0800
+++ linux/drivers/gpu/drm/i915/intel_dp.c   2011-11-16 21:37:00.0 
+0800
@@ -1984,6 +1984,22 @@ intel_dp_detect(struct drm_connector *co
return connector_status_connected;
 }
 
+static void intel_dp_hot_remove(struct drm_connector *connector)
+{
+   struct intel_dp *intel_dp = intel_attached_dp(connector);
+   struct drm_device *dev = intel_dp->base.base.dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct drm_crtc *crtc = intel_dp->base.base.crtc;
+
+   intel_dp->DP &= ~DP_AUDIO_OUTPUT_ENABLE;
+   I915_WRITE(intel_dp->output_reg, intel_dp->DP);
+   POSTING_READ(intel_dp->output_reg);
+
+   connector->eld[0] = 0;
+   if (dev_priv->display.write_eld)
+   dev_priv->display.write_eld(connector, crtc);
+}
+
 static int intel_dp_get_modes(struct drm_connector *connector)
 {
struct intel_dp *intel_dp = intel_attached_dp(connector);
@@ -2143,6 +2159,7 @@ static const struct drm_connector_funcs 
.detect = intel_dp_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
.set_property = intel_dp_set_property,
+   .hot_remove = intel_dp_hot_remove,
.destroy = intel_dp_destroy,
 };
 
--- linux.orig/drivers/gpu/drm/i915/intel_hdmi.c2011-11-16 
21:36:58.0 +0800
+++ linux/drivers/gpu/drm/i915/intel_hdmi.c 2011-11-16 21:37:00.0 
+0800
@@ -350,6 +350,22 @@ intel_hdmi_detect(struct drm_connector *
return status;
 }
 
+static void intel_hdmi_hot_remove(struct drm_connector *connector)
+{
+   struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
+   struct drm_i915_private *dev_priv = connector->dev->dev_private;
+   u32 temp;
+
+   temp = I915_READ(intel_hdmi->sdvox_reg);
+   I915_WRITE(intel_hdmi->sdvox_reg, temp & ~SDVO_AUDIO_ENABLE);
+   POSTING_READ(intel_hdmi->sdvox_reg);
+
+   connector->eld[0] = 0;
+   if (dev_priv->display.write_eld)
+   dev_priv->display.write_eld(connector,
+   intel_hdmi->base.base.crtc);
+}
+
 static int intel_hdmi_get_modes(struct drm_connector *connector)
 {
struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
@@ -459,6 +475,7 @@ static const struct drm_connector_funcs 
.detect = intel_hdmi_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
.set_property = intel_hdmi_set_property,
+   .hot_remove = intel_hdmi_hot_remove,
.destroy = intel_hdmi_destroy,
 };
 
--- linux.orig/drivers/gpu/drm/drm_crtc_helper.c2011-11-16 
21:36:58.0 +0800
+++ linux/drivers/gpu/drm/drm_crtc_helper.c 2011-11-16 21:37:00.0 
+0800
@@ -905,6 +905,10 @@ static void output_poll_execute(struct w
  old_status, connector->status);
if (old_status != connector->status)
changed = true;
+   if (old_status == connector_status_connected &&
+   connector->status == connector_status_disconnected)
+   connector->funcs->hot_remove(connector);
+
}
 
mutex_unlock(&dev->mode_config.mutex);
--- linux.orig/include/drm/drm_crtc.h   2011-11-16 21:36:58.0 +0800
+++ linux/include/drm/drm_crtc.h2011-11-16 21:37:00.0 +0800
@@ -419,6 +419,7 @@ struct drm_connector_funcs {
int (*fill_modes)(struct drm_connector *connector, uint32_t max_width, 
uint32_t max_height);
int (*set_property)(struct drm_connector *connector, struct 
drm_property *property,
 uint64_t val);
+   void (*hot_remove)(struct drm_connector *connector);
void (*destroy)(struct drm_connector *connector);
void (*force)(struct drm_connector *connector);
 };
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 3/3] drm/i915: hot removal notification to HDMI audio driver

2011-11-16 Thread Wu Fengguang
Sorry forgot to remove this left over chunk...

Note that I've not yet got the hardware to test the DisplayPort part
of this patch, but should be able to do so this week.

> --- linux.orig/drivers/gpu/drm/i915/intel_drv.h   2011-11-16 
> 20:54:27.0 +0800
> +++ linux/drivers/gpu/drm/i915/intel_drv.h2011-11-16 21:19:42.0 
> +0800
> @@ -382,6 +382,10 @@ extern void intel_fb_restore_mode(struct
>  extern void intel_init_clock_gating(struct drm_device *dev);
>  extern void intel_write_eld(struct drm_encoder *encoder,
>   struct drm_display_mode *mode);
> +extern void intel_hotplug_status(struct drm_device *dev,
> +   struct drm_connector *connector,
> +   struct drm_crtc *crtc,
> +   enum drm_connector_status status);
>  extern void intel_cpt_verify_modeset(struct drm_device *dev, int pipe);
>  
>  #endif /* __INTEL_DRV_H__ */
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 3/3] drm/i915: hot removal notification to HDMI audio driver

2011-11-16 Thread Wu Fengguang
On monitor hot removal:

1) clear SDVO_AUDIO_ENABLE or DP_AUDIO_OUTPUT_ENABLE
2) clear ELD Valid bit

So that the audio driver will receive hot plug events and take action to
refresh its device state and ELD contents.

cc: Wang Zhenyu 
Signed-off-by: Wu Fengguang 
---
 drivers/gpu/drm/drm_crtc_helper.c |4 
 drivers/gpu/drm/i915/intel_dp.c   |   17 +
 drivers/gpu/drm/i915/intel_drv.h  |4 
 drivers/gpu/drm/i915/intel_hdmi.c |   17 +
 include/drm/drm_crtc.h|1 +
 5 files changed, 43 insertions(+)

--- linux.orig/drivers/gpu/drm/i915/intel_dp.c  2011-11-16 20:54:28.0 
+0800
+++ linux/drivers/gpu/drm/i915/intel_dp.c   2011-11-16 21:19:42.0 
+0800
@@ -1984,6 +1984,22 @@ intel_dp_detect(struct drm_connector *co
return connector_status_connected;
 }
 
+static void intel_dp_hot_remove(struct drm_connector *connector)
+{
+   struct intel_dp *intel_dp = intel_attached_dp(connector);
+   struct drm_device *dev = intel_dp->base.base.dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct drm_crtc *crtc = intel_dp->base.base.crtc;
+
+   intel_dp->DP &= ~DP_AUDIO_OUTPUT_ENABLE;
+   I915_WRITE(intel_dp->output_reg, intel_dp->DP);
+   POSTING_READ(intel_dp->output_reg);
+
+   connector->eld[0] = 0;
+   if (dev_priv->display.write_eld)
+   dev_priv->display.write_eld(connector, crtc);
+}
+
 static int intel_dp_get_modes(struct drm_connector *connector)
 {
struct intel_dp *intel_dp = intel_attached_dp(connector);
@@ -2143,6 +2159,7 @@ static const struct drm_connector_funcs 
.detect = intel_dp_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
.set_property = intel_dp_set_property,
+   .hot_remove = intel_dp_hot_remove,
.destroy = intel_dp_destroy,
 };
 
--- linux.orig/drivers/gpu/drm/i915/intel_drv.h 2011-11-16 20:54:27.0 
+0800
+++ linux/drivers/gpu/drm/i915/intel_drv.h  2011-11-16 21:19:42.0 
+0800
@@ -382,6 +382,10 @@ extern void intel_fb_restore_mode(struct
 extern void intel_init_clock_gating(struct drm_device *dev);
 extern void intel_write_eld(struct drm_encoder *encoder,
struct drm_display_mode *mode);
+extern void intel_hotplug_status(struct drm_device *dev,
+ struct drm_connector *connector,
+ struct drm_crtc *crtc,
+ enum drm_connector_status status);
 extern void intel_cpt_verify_modeset(struct drm_device *dev, int pipe);
 
 #endif /* __INTEL_DRV_H__ */
--- linux.orig/drivers/gpu/drm/i915/intel_hdmi.c2011-11-16 
20:55:13.0 +0800
+++ linux/drivers/gpu/drm/i915/intel_hdmi.c 2011-11-16 21:19:42.0 
+0800
@@ -350,6 +350,22 @@ intel_hdmi_detect(struct drm_connector *
return status;
 }
 
+static void intel_hdmi_hot_remove(struct drm_connector *connector)
+{
+   struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
+   struct drm_i915_private *dev_priv = connector->dev->dev_private;
+   u32 temp;
+
+   temp = I915_READ(intel_hdmi->sdvox_reg);
+   I915_WRITE(intel_hdmi->sdvox_reg, temp & ~SDVO_AUDIO_ENABLE);
+   POSTING_READ(intel_hdmi->sdvox_reg);
+
+   connector->eld[0] = 0;
+   if (dev_priv->display.write_eld)
+   dev_priv->display.write_eld(connector,
+   intel_hdmi->base.base.crtc);
+}
+
 static int intel_hdmi_get_modes(struct drm_connector *connector)
 {
struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
@@ -459,6 +475,7 @@ static const struct drm_connector_funcs 
.detect = intel_hdmi_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
.set_property = intel_hdmi_set_property,
+   .hot_remove = intel_hdmi_hot_remove,
.destroy = intel_hdmi_destroy,
 };
 
--- linux.orig/drivers/gpu/drm/drm_crtc_helper.c2011-11-16 
20:55:13.0 +0800
+++ linux/drivers/gpu/drm/drm_crtc_helper.c 2011-11-16 21:19:42.0 
+0800
@@ -905,6 +905,10 @@ static void output_poll_execute(struct w
  old_status, connector->status);
if (old_status != connector->status)
changed = true;
+   if (old_status == connector_status_connected &&
+   connector->status == connector_status_disconnected)
+   connector->funcs->hot_remove(connector);
+
}
 
mutex_unlock(&dev->mode_config.mutex);
--- linux.orig/include/drm/drm_crtc.h   2011-11-16 20:54:28.0 +0800
+++ linux/include/drm/drm_crtc.h2011-11-16 21:19:42.0 +0800
@@ -419,6 +419,7 @@ struct drm_connector_funcs {
int (*fill_modes)(struct drm_connector *connector, uint32_t max_width, 
uint32_t max_height);
int (*set_property)(struct drm_connector *connector, struct 
drm_property *property,
 

[Intel-gfx] [PATCH 1/3] drm/i915: fix ELD writing for SandyBridge

2011-11-16 Thread Wu Fengguang
SandyBridge should be using the same register addresses as IvyBridge.

Signed-off-by: Wu Fengguang 
---
 drivers/gpu/drm/i915/i915_reg.h  |6 +++---
 drivers/gpu/drm/i915/intel_display.c |   10 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

--- linux.orig/drivers/gpu/drm/i915/i915_reg.h  2011-11-09 13:17:19.0 
+0800
+++ linux/drivers/gpu/drm/i915/i915_reg.h   2011-11-09 13:18:39.0 
+0800
@@ -3543,8 +3543,8 @@
 #define GEN5_ELD_VALIDB(1 << 0)
 #define GEN5_CP_READYB (1 << 1)
 
-#define GEN7_HDMIW_HDMIEDID_A  0xE5050
-#define GEN7_AUD_CNTRL_ST_A0xE50B4
-#define GEN7_AUD_CNTRL_ST2 0xE50C0
+#define GEN6_HDMIW_HDMIEDID_A  0xE5050
+#define GEN6_AUD_CNTL_ST_A 0xE50B4
+#define GEN6_AUD_CNTRL_ST2 0xE50C0
 
 #endif /* _I915_REG_H_ */
--- linux.orig/drivers/gpu/drm/i915/intel_display.c 2011-11-09 
13:19:28.0 +0800
+++ linux/drivers/gpu/drm/i915/intel_display.c  2011-11-09 13:20:02.0 
+0800
@@ -5857,14 +5857,14 @@ static void ironlake_write_eld(struct dr
int aud_cntl_st;
int aud_cntrl_st2;
 
-   if (IS_IVYBRIDGE(connector->dev)) {
-   hdmiw_hdmiedid = GEN7_HDMIW_HDMIEDID_A;
-   aud_cntl_st = GEN7_AUD_CNTRL_ST_A;
-   aud_cntrl_st2 = GEN7_AUD_CNTRL_ST2;
-   } else {
+   if (IS_GEN5(connector->dev)) {
hdmiw_hdmiedid = GEN5_HDMIW_HDMIEDID_A;
aud_cntl_st = GEN5_AUD_CNTL_ST_A;
aud_cntrl_st2 = GEN5_AUD_CNTL_ST2;
+   } else {
+   hdmiw_hdmiedid = GEN6_HDMIW_HDMIEDID_A;
+   aud_cntl_st = GEN6_AUD_CNTL_ST_A;
+   aud_cntrl_st2 = GEN6_AUD_CNTRL_ST2;
}
 
i = to_intel_crtc(crtc)->pipe;


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


[Intel-gfx] [PATCH 2/3] drm/i915: dont trigger hotplug events on unchanged ELD

2011-11-16 Thread Wu Fengguang
The ELD may or may not change when switching the video mode.
If unchanged, don't trigger hot plug events to HDMI audio driver.

This avoids disturbing the user with repeated printks.

Reported-by: Nick Bowler 
Signed-off-by: Wu Fengguang 
---
 drivers/gpu/drm/i915/intel_display.c |   51 ++---
 1 file changed, 46 insertions(+), 5 deletions(-)

--- linux.orig/drivers/gpu/drm/i915/intel_display.c 2011-11-10 
17:23:04.0 +0800
+++ linux/drivers/gpu/drm/i915/intel_display.c  2011-11-10 17:59:25.0 
+0800
@@ -5811,6 +5811,35 @@ static int intel_crtc_mode_set(struct dr
return ret;
 }
 
+static bool intel_eld_uptodate(struct drm_connector *connector,
+  int reg_eldv, uint32_t bits_eldv,
+  int reg_elda, uint32_t bits_elda,
+  int reg_edid)
+{
+   struct drm_i915_private *dev_priv = connector->dev->dev_private;
+   uint8_t *eld = connector->eld;
+   uint32_t i;
+
+   i = I915_READ(reg_eldv);
+   i &= bits_eldv;
+
+   if (!eld[0])
+   return !i;
+
+   if (!i)
+   return false;
+
+   i = I915_READ(reg_elda);
+   i &= ~bits_elda;
+   I915_WRITE(reg_elda, i);
+
+   for (i = 0; i < eld[2]; i++)
+   if (I915_READ(reg_edid) != *((uint32_t *)eld + i))
+   return false;
+
+   return true;
+}
+
 static void g4x_write_eld(struct drm_connector *connector,
  struct drm_crtc *crtc)
 {
@@ -5827,6 +5856,12 @@ static void g4x_write_eld(struct drm_con
else
eldv = G4X_ELDV_DEVCTG;
 
+   if (intel_eld_uptodate(connector,
+  G4X_AUD_CNTL_ST, eldv,
+  G4X_AUD_CNTL_ST, G4X_ELD_ADDR,
+  G4X_HDMIW_HDMIEDID))
+   return;
+
i = I915_READ(G4X_AUD_CNTL_ST);
i &= ~(eldv | G4X_ELD_ADDR);
len = (i >> 9) & 0x1f;  /* ELD buffer size */
@@ -5886,6 +5921,17 @@ static void ironlake_write_eld(struct dr
eldv = GEN5_ELD_VALIDB << ((i - 1) * 4);
}
 
+   if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) {
+   DRM_DEBUG_DRIVER("ELD: DisplayPort detected\n");
+   eld[5] |= (1 << 2); /* Conn_Type, 0x1 = DisplayPort */
+   }
+
+   if (intel_eld_uptodate(connector,
+  aud_cntrl_st2, eldv,
+  aud_cntl_st, GEN5_ELD_ADDRESS,
+  hdmiw_hdmiedid))
+   return;
+
i = I915_READ(aud_cntrl_st2);
i &= ~eldv;
I915_WRITE(aud_cntrl_st2, i);
@@ -5893,11 +5939,6 @@ static void ironlake_write_eld(struct dr
if (!eld[0])
return;
 
-   if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) {
-   DRM_DEBUG_DRIVER("ELD: DisplayPort detected\n");
-   eld[5] |= (1 << 2); /* Conn_Type, 0x1 = DisplayPort */
-   }
-
i = I915_READ(aud_cntl_st);
i &= ~GEN5_ELD_ADDRESS;
I915_WRITE(aud_cntl_st, i);


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


[Intel-gfx] [PATCH 0/3] HDMI ELD fixes for 3.2

2011-11-16 Thread Wu Fengguang
Keith,

Here are 3 fixes on HDMI/ELD audio.

The third one adds a ->hot_remove hook to drm_connector_funcs. Please review.

[PATCH 1/3] drm/i915: fix ELD writing for SandyBridge
[PATCH 2/3] drm/i915: dont trigger hotplug events on unchanged ELD
[PATCH 3/3] drm/i915: hot removal notification to HDMI audio driver

Thanks,
Fengguang

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


Re: [Intel-gfx] [PATCH v3] drm/i915: Honor SSC quirk table over the default, unless set by user

2011-11-16 Thread Michel Alexandre Salim
Hi Keith,

That patch is still not in 3.2-rc2, drm-intel-fixes or drm-intel-next.
I've been using it successfully on i915 (both SSC-blacklisted and not)
and non-i915 machines; feel free to set the Tested-by and Reviewed-by tags.

Thanks,

-- 
Michel

On 11/09/2011 07:07 PM, Keith Packard wrote:
> On Wed, 09 Nov 2011 17:30:29 +0100, Michel Alexandre Salim 
>  wrote:
>> Additional note: while I've not touched the line since it does not
>> affect me, it seems that i915_panel_use_ssc *cannot* be less than 0
>> since that variable is declared as unsigned.
> 
> Oops. That's the bug here -- we're supposed to make it so that the
> command line can override the quirks, but there's no way to use a quirk
> given the mis-declared parameter.
> 
> This is untested...
> 
> From e64ecadef40e3c2035cd4e9b967ffd83489bdea0 Mon Sep 17 00:00:00 2001
> From: Keith Packard 
> Date: Wed, 9 Nov 2011 09:57:50 -0800
> Subject: [PATCH] drm/i915: Module parameters using '-1' as default must be
>  signed type
> 
> Testing i915_panel_use_ssc for the default value was broken, so the
> driver would never autodetect the correct value.
> 
> Signed-off-by: Keith Packard 
> ---
>  drivers/gpu/drm/i915/i915_drv.c |4 ++--
>  drivers/gpu/drm/i915/i915_drv.h |4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 548e04b..13488be 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -67,7 +67,7 @@ module_param_named(i915_enable_rc6, i915_enable_rc6, int, 
> 0600);
>  MODULE_PARM_DESC(i915_enable_rc6,
>   "Enable power-saving render C-state 6 (default: true)");
>  
> -unsigned int i915_enable_fbc __read_mostly = -1;
> +int i915_enable_fbc __read_mostly = -1;
>  module_param_named(i915_enable_fbc, i915_enable_fbc, int, 0600);
>  MODULE_PARM_DESC(i915_enable_fbc,
>   "Enable frame buffer compression for power savings "
> @@ -79,7 +79,7 @@ MODULE_PARM_DESC(lvds_downclock,
>   "Use panel (LVDS/eDP) downclocking for power savings "
>   "(default: false)");
>  
> -unsigned int i915_panel_use_ssc __read_mostly = -1;
> +int i915_panel_use_ssc __read_mostly = -1;
>  module_param_named(lvds_use_ssc, i915_panel_use_ssc, int, 0600);
>  MODULE_PARM_DESC(lvds_use_ssc,
>   "Use Spread Spectrum Clock with panels [LVDS/eDP] "
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d2da91f..4a9c1b9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1000,10 +1000,10 @@ extern int i915_panel_ignore_lid __read_mostly;
>  extern unsigned int i915_powersave __read_mostly;
>  extern unsigned int i915_semaphores __read_mostly;
>  extern unsigned int i915_lvds_downclock __read_mostly;
> -extern unsigned int i915_panel_use_ssc __read_mostly;
> +extern int i915_panel_use_ssc __read_mostly;
>  extern int i915_vbt_sdvo_panel_type __read_mostly;
>  extern unsigned int i915_enable_rc6 __read_mostly;
> -extern unsigned int i915_enable_fbc __read_mostly;
> +extern int i915_enable_fbc __read_mostly;
>  extern bool i915_enable_hangcheck __read_mostly;
>  
>  extern int i915_suspend(struct drm_device *dev, pm_message_t state);
> 
> 
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx


-- 
Michel Alexandre Salim
µblog:  http://identi.ca/hircus
http://twitter.com/hircus
GPG key ID: 78884778

()  ascii ribbon campaign - against html e-mail
/\  www.asciiribbon.org   - against proprietary attachments
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/2 v2] hda - delayed ELD repoll

2011-11-16 Thread Paul Menzel
Dear Fengguang,


Am Mittwoch, den 16.11.2011, 00:57 +0800 schrieb Wu Fengguang:
> The Intel HDMI chips (ironlake at least) are found to have ~250ms delay
> between the ELD_Valid=1 hotplug event is send and the ELD buffer becomes
> actually readable. During the time the ELD buffer is mysteriously all 0.
> 
> Fix it by scheduling a delayed work to re-read ELD buffer after 300ms.
> 
> Signed-off-by: Wu Fengguang 

other then my comment at the end, please note that it is very confusing
for European (and other folks) what your first name is.

If it is Fengguang then you should use

Fengguang Wu or Wu, Fengguang

where I prefer the first one.

> ---
>  sound/pci/hda/hda_local.h  |2 +
>  sound/pci/hda/patch_hdmi.c |   49 ++-
>  2 files changed, 44 insertions(+), 7 deletions(-)
> 
> --- linux.orig/sound/pci/hda/hda_local.h  2011-11-15 21:29:53.0 
> +0800
> +++ linux/sound/pci/hda/hda_local.h   2011-11-15 21:29:54.0 +0800
> @@ -655,6 +655,8 @@ struct hdmi_eld {
>  #ifdef CONFIG_PROC_FS
>   struct snd_info_entry *proc_entry;
>  #endif
> + struct hda_codec *codec;
> + struct delayed_work work;
>  };
>  
>  int snd_hdmi_get_eld_size(struct hda_codec *codec, hda_nid_t nid);
> --- linux.orig/sound/pci/hda/patch_hdmi.c 2011-11-15 21:29:53.0 
> +0800
> +++ linux/sound/pci/hda/patch_hdmi.c  2011-11-16 00:50:50.0 +0800
> @@ -746,7 +746,7 @@ static void hdmi_setup_audio_infoframe(s
>   */
>  
>  static void hdmi_present_sense(struct hda_codec *codec, hda_nid_t pin_nid,
> -struct hdmi_eld *eld);
> +struct hdmi_eld *eld, bool retry);
>  
>  static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
>  {
> @@ -766,7 +766,7 @@ static void hdmi_intrinsic_event(struct 
>   return;
>   eld = &spec->pins[pin_idx].sink_eld;
>  
> - hdmi_present_sense(codec, pin_nid, eld);
> + hdmi_present_sense(codec, pin_nid, eld, true);
>  
>   /*
>* HDMI sink's ELD info cannot always be retrieved for now, e.g.
> @@ -969,7 +969,7 @@ static int hdmi_read_pin_conn(struct hda
>  }
>  
>  static void hdmi_present_sense(struct hda_codec *codec, hda_nid_t pin_nid,
> -struct hdmi_eld *eld)
> +struct hdmi_eld *eld, bool retry)
>  {
>   /*
>* Always execute a GetPinSense verb here, even when called from
> @@ -992,13 +992,31 @@ static void hdmi_present_sense(struct hd
>   "HDMI status: Codec=%d Pin=%d Presence_Detect=%d 
> ELD_Valid=%d\n",
>   codec->addr, pin_nid, eld->monitor_present, eld_valid);
>  
> - if (eld_valid)
> + if (eld_valid) {
>   if (!snd_hdmi_get_eld(eld, codec, pin_nid))
>   snd_hdmi_show_eld(eld);
> + else if (retry) {
> + queue_delayed_work(codec->bus->workq,
> +&eld->work,
> +msecs_to_jiffies(300));
> + }

Would adding a comment referring to the discussion thread be beneficial
for code readers asking why a delay is needed?

> + }
>  
>   snd_hda_input_jack_report(codec, pin_nid);
>  }

[…]


Thanks,

Paul


signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Fix inconsistent backlight level during disabled

2011-11-16 Thread Takashi Iwai
When the brightness property is inquired while the backlight is disabled,
the driver returns a wrong value (zero) because it probes the value after
the backlight was turned off.  This caused a black screen even after the
backlight is enabled again.  It should return the internal backlight_level
instead, so that it won't be influenced by the backlight-enable state.

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=41926
BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/872652

Tested-by: Kamal Mostafa 
Cc: Alex Davis 
Cc: 
Signed-off-by: Takashi Iwai 
---
 drivers/gpu/drm/i915/intel_panel.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_panel.c 
b/drivers/gpu/drm/i915/intel_panel.c
index 499d4c0..21f60b7 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -326,7 +326,8 @@ static int intel_panel_update_status(struct 
backlight_device *bd)
 static int intel_panel_get_brightness(struct backlight_device *bd)
 {
struct drm_device *dev = bl_get_data(bd);
-   return intel_panel_get_backlight(dev);
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   return dev_priv->backlight_level;
 }
 
 static const struct backlight_ops intel_panel_bl_ops = {
-- 
1.7.7

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


Re: [Intel-gfx] [PATCH] i915: Fix bug where screen brightness is not restored

2011-11-16 Thread Takashi Iwai
At Tue, 15 Nov 2011 13:54:44 -0800,
Kamal Mostafa wrote:
> 
> On Tue, 2011-11-15 at 21:28 +0100, Takashi Iwai wrote:
> > My rough guess is the inconsistency of property taken during the
> > backlight disabled.  How about the patch below (untested!) in addition
> > to the fix in 3.2?
> > 
> > 
> > Takashi
> 
> Yes Takashi, your patch below (plus the already committed fix[0]) does
> indeed fix the problem[1] for me.  Thanks!
> 
> Tested-by: Kamal Mostafa 

OK, I'll resend the patch with proper sign-off, etc.

thanks,

Takashi

> 
>  -Kamal
> 
> [0] f52c619a590fa75276c07dfcaf380dee53e4ea4c
> drm/i915/panel: Always record the backlight level again (but cleverly)
> 
> [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/872652
> 
> 
> 
> > ---
> > diff --git a/drivers/gpu/drm/i915/intel_panel.c 
> > b/drivers/gpu/drm/i915/intel_panel.c
> > index 499d4c0..21f60b7 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -326,7 +326,8 @@ static int intel_panel_update_status(struct 
> > backlight_device *bd)
> >  static int intel_panel_get_brightness(struct backlight_device *bd)
> >  {
> > struct drm_device *dev = bl_get_data(bd);
> > -   return intel_panel_get_backlight(dev);
> > +   struct drm_i915_private *dev_priv = dev->dev_private;
> > +   return dev_priv->backlight_level;
> >  }
> >  
> >  static const struct backlight_ops intel_panel_bl_ops = {
> > 
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx