Re: [Intel-gfx] [PATCH 1/2] drm/i915: Send a DPIO cmnreset during driver load or system resume.

2013-09-25 Thread Ville Syrjälä
On Wed, Sep 25, 2013 at 07:14:34AM +0800, Lee, Chon Ming wrote:
 On 09/24 18:18, Ville Syrjälä wrote:
  On Tue, Sep 24, 2013 at 03:14:27PM +0800, Chon Ming Lee wrote:
   Without the DPIO cmnreset, the PLL fail to lock.  This should have
   done by BIOS.
   
   v2: Move this to intel_uncore_sanitize to allow it to get call during
   resume path. (Daniel)
   v3: Remove redundant write 0 to DPIO_CTL, and use DPIO_RESET instead of
   just 0x1 (Ville)
   Without BIOS, DPIO/render well/media well may still power gated.
   Turn it off.
   
   Signed-off-by: Chon Ming Lee chon.ming@intel.com
   ---
drivers/gpu/drm/i915/i915_reg.h |9 +
drivers/gpu/drm/i915/intel_uncore.c |   23 +++
2 files changed, 32 insertions(+), 0 deletions(-)
   
   diff --git a/drivers/gpu/drm/i915/i915_reg.h 
   b/drivers/gpu/drm/i915/i915_reg.h
   index c4f9bef..c02f893 100644
   --- a/drivers/gpu/drm/i915/i915_reg.h
   +++ b/drivers/gpu/drm/i915/i915_reg.h
   @@ -361,6 +361,15 @@
#define PUNIT_OPCODE_REG_READ6
#define PUNIT_OPCODE_REG_WRITE   7

   +#define PUNIT_REG_PWRGT_CTRL 0x60
   +#define PUNIT_REG_PWRGT_STATUS   0x61
   +#definePUNIT_CLK_GATE1
   +#definePUNIT_PWR_RESET   2
   +#definePUNIT_PWR_GATE3
   +#defineRENDER_PWRGT  (PUNIT_PWR_GATE  0)
   +#defineMEDIA_PWRGT   (PUNIT_PWR_GATE  2)
   +#defineDPIO_PWRGT(PUNIT_PWR_GATE  6)
  
  Subsys 6 seems to be one of four TX lanes, and there's also the common
  lane subsys, and the disp2d is one as well. RX supposedly is not relevant
  for display PHY, not sure why it has subsys allocated too.
  
  Anyways my point would be that shouldn't we check all subsys ie render + 
  media +
  disp2d + common lane + all tx lanes?
  
 By default, the common lane + all tx lanes are not power gated during cold 
 boot
 or system resume.  Unless S0ix entry actually power gate it by driver, then,
 this will need to add to turn off it.  

OK. And as Imre pointed out to me the ' 6' isn't a TX lane as I
claimed but the display subsystems (3). I assume that's the same thing
as the disp2d block, ie. the pipes. So the DPIO in the name is
wrong. It should be called display or disp2d I think.

If you say the PHY side isn't power gated during cold boot, I think we
can ignore it for now.

So if you rename the DPIO thing, this patch should be OK.

 
  And should we maybe power gate the RX lanes always as those shouldn't be 
  needed
  for display?
 
 Yes, you are correct.  I believe there should be another patch to do it, to
 enable power gate the VLV correctly for SOix entry or exit.  

My plan is to power gate everything we can during runtime, not just s0ix.
But that's a biger topic we can discuss once Imre gets some relevant
patches ready.

  
   +
#define PUNIT_REG_GPU_LFM0xd3
#define PUNIT_REG_GPU_FREQ_REQ   0xd4
#define PUNIT_REG_GPU_FREQ_STS   0xd8
   diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
   b/drivers/gpu/drm/i915/intel_uncore.c
   index 8649f1c..6923b4d 100644
   --- a/drivers/gpu/drm/i915/intel_uncore.c
   +++ b/drivers/gpu/drm/i915/intel_uncore.c
   @@ -276,10 +276,33 @@ static void intel_uncore_forcewake_reset(struct 
   drm_device *dev)

void intel_uncore_sanitize(struct drm_device *dev)
{
   + struct drm_i915_private *dev_priv = dev-dev_private;
   + u32 reg_val;
   +
 intel_uncore_forcewake_reset(dev);

 /* BIOS often leaves RC6 enabled, but disable it for hw init */
 intel_disable_gt_powersave(dev);
   +
   + /* Trigger DPIO CMN RESET and turn off power gate, require
   +  * especially in BIOS less system
   +  */
   + if (IS_VALLEYVIEW(dev)) {
   +
   + mutex_lock(dev_priv-rps.hw_lock);
   + reg_val = vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_STATUS);
   +
   + if (reg_val  (RENDER_PWRGT | MEDIA_PWRGT | DPIO_PWRGT))
   + vlv_punit_write(dev_priv, PUNIT_REG_PWRGT_CTRL, 0x0);
   +
   + mutex_unlock(dev_priv-rps.hw_lock);
   +
   + reg_val = I915_READ(DPIO_CTL);
   + if (!(reg_val  DPIO_RESET)) {
   + I915_WRITE(DPIO_CTL, DPIO_RESET);
   + POSTING_READ(DPIO_CTL);
   + }
   + }
}

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

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


[Intel-gfx] [PATCH] drm/i915: Fix VLV eDP timing

2013-09-25 Thread Chon Ming Lee
Fix the typo in previous commit for DP 1.62 divisor.

drm/i915: Move Valleyview DP DPLL divisor calc to intel_dp_set_clock v2

Reported-by: Jesse Barnes jbar...@virtuousgeek.org 
Signed-off-by: Chon Ming Lee chon.ming@intel.com
---
 drivers/gpu/drm/i915/intel_dp.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5e1de35..a8ceccf 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -59,7 +59,7 @@ static const struct dp_link_dpll pch_dpll[] = {
 
 static const struct dp_link_dpll vlv_dpll[] = {
{ DP_LINK_BW_1_62,
-   { .p1 = 3, .p2 = 2, .n = 5, .m1 = 5, .m2 = 3 } },
+   { .p1 = 3, .p2 = 2, .n = 5, .m1 = 5, .m2 = 81 } },
{ DP_LINK_BW_2_7,
{ .p1 = 2, .p2 = 2, .n = 1, .m1 = 2, .m2 = 27 } }
 };
-- 
1.7.7.6

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


[Intel-gfx] [PATCH] drm/i915: Fix 1.62 DP DPLL settings for VLV

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

Use the recommended PLL dividers from
VLV2_DPLL_mphy_hsdpll_frequency_table_ww6_rev1p1.xlsm. The previous
values were really bogus. The 2.7 values look good however.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5e1de35..a5e4e61 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -59,7 +59,7 @@ static const struct dp_link_dpll pch_dpll[] = {
 
 static const struct dp_link_dpll vlv_dpll[] = {
{ DP_LINK_BW_1_62,
-   { .p1 = 3, .p2 = 2, .n = 5, .m1 = 5, .m2 = 3 } },
+   { .p1 = 3, .p2 = 2, .n = 5, .m1 = 3, .m2 = 81 } },
{ DP_LINK_BW_2_7,
{ .p1 = 2, .p2 = 2, .n = 1, .m1 = 2, .m2 = 27 } }
 };
-- 
1.8.1.5

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


[Intel-gfx] [PATCH] drm/i915: Fix VLV eDP timing v2

2013-09-25 Thread Chon Ming Lee
Fix the typo in previous commit for DP 1.62 divisor.
drm/i915: Move Valleyview DP DPLL divisor calc to intel_dp_set_clock v2

v2: sigh, the m1 div is 3.

Reported-by: Jesse Barnes jbar...@virtuousgeek.org 
Signed-off-by: Chon Ming Lee chon.ming@intel.com
---
 drivers/gpu/drm/i915/intel_dp.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5e1de35..a5e4e61 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -59,7 +59,7 @@ static const struct dp_link_dpll pch_dpll[] = {
 
 static const struct dp_link_dpll vlv_dpll[] = {
{ DP_LINK_BW_1_62,
-   { .p1 = 3, .p2 = 2, .n = 5, .m1 = 5, .m2 = 3 } },
+   { .p1 = 3, .p2 = 2, .n = 5, .m1 = 3, .m2 = 81 } },
{ DP_LINK_BW_2_7,
{ .p1 = 2, .p2 = 2, .n = 1, .m1 = 2, .m2 = 27 } }
 };
-- 
1.7.7.6

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


Re: [Intel-gfx] BUG: sleeping function called from invalid context on 3.10.10-rt7

2013-09-25 Thread Ville Syrjälä
On Wed, Sep 25, 2013 at 06:32:10AM +0200, Mario Kleiner wrote:
 On 23.09.13 10:38, Ville Syrjälä wrote:
  On Sat, Sep 21, 2013 at 12:07:36AM +0200, Mario Kleiner wrote:
  On 09/17/2013 10:55 PM, Daniel Vetter wrote:
  On Tue, Sep 17, 2013 at 9:50 PM, Peter Hurley pe...@hurleysoftware.com 
  wrote:
  On 09/11/2013 03:31 PM, Peter Hurley wrote:
 
  [+cc dri-devel]
 
  On 09/11/2013 11:38 AM, Steven Rostedt wrote:
 
  On Wed, 11 Sep 2013 11:16:43 -0400
  Peter Hurley pe...@hurleysoftware.com wrote:
 
  The funny part is, there's a comment there that shows that this was
  done even for PREEMPT_RT. Unfortunately, the call to
  get_scanout_position() can call functions that use the rt-mutex
  sleeping spin locks and it breaks there.
 
  I guess we need to ask the authors of the mainline patch exactly why
  that preempt_disable() is needed?
 
 
  The drm core associates a timestamp with each vertical blank frame #.
  Drm drivers can optionally support a 'high resolution' hw timestamp.
  The vblank frame #/timestamp tuple is user-space visible.
 
  The i915 drm driver supports a hw timestamp via this drm helper 
  function
  which computes the timestamp from the crtc scan position (based on the
  pixel clock).
 
  For mainline, the preempt_disable/_enable() isn't actually necessary
  because every call tree that leads here already has preemption 
  disabled.
 
  For -RT, the maybe i915 register spinlock (uncore.lock) should be raw?
 
 
  No, it should not. Note, any other lock that can be held when it is
  held would also need to be raw.
 
 
  By that, you mean any other lock that might be claimed would also 
  need
  to be raw?  Hopefully not any other lock already held?
 
  And by taking a quick audit of the code, I see this:
 
spin_lock_irqsave(dev_priv-uncore.lock, irqflags);
 
/* Reset the chip */
 
/* GEN6_GDRST is not in the gt power well, no need to check
 * for fifo space for the write or forcewake the chip for
 * the read
 */
__raw_i915_write32(dev_priv, GEN6_GDRST, GEN6_GRDOM_FULL);
 
/* Spin waiting for the device to ack the reset request */
ret = wait_for((__raw_i915_read32(dev_priv, GEN6_GDRST) 
  GEN6_GRDOM_FULL) == 0, 500);
 
  That spin is unacceptable in RT with preemption and interrupts 
  disabled.
 
 
  Yep. That would be bad.
 
  AFAICT the registers read in i915_get_crtc_scanoutpos() aren't included
  in the force-wake set, so raw reads of the registers would
  probably be acceptable (thus obviating the need for claiming the
  uncore.lock).
 
  Except that _ALL_ register access is disabled with the uncore.lock
  during a gpu reset. Not sure if that's meant to include crtc registers
  or not, or what other synchronization/serialization issues are being
  handled/hidden by forcing all register accesses to wait during a gpu
  reset.
 
  Hopefully an i915 expert can weigh in here?
 
 
 
  Daniel,
 
  Can you shed some light on whether the i915+ crtc registers (specifically
  those in i915_get_crtc_scanoutpos() and i915_/gm45_get_vblank_counter())
  read as part of the vblank counter/timestamp handling need to
  be prevented during gpu reset?
 
  The depency here in the locking is a recent addition:
 
  commit a7cd1b8fea2f341b626b255d9898a5ca5fabbf0a
  Author: Chris Wilson ch...@chris-wilson.co.uk
  Date:   Fri Jul 19 20:36:51 2013 +0100
 
drm/i915: Serialize almost all register access
 
  It's a (slightly) oversized hammer to work around a hardware issue -
  we could break it down to register blocks, which can be accessed
  concurrently, but that tends to be more fragile. But the chip really
  dies if you access (even just reads) the same block concurrently :(
 
  We could try break the spinlock protected section a bit in the reset
  handler - register access on a hung gpu tends to be ill-defined
  anyway.
 
  The implied wait with preemption and interrupts disabled is causing grief
  in -RT, but also a 4ms wait inside an irq handler seems like a bad idea.
 
  Oops, the magic code in wait_for which is just there to make the imo
  totally misguided kgdb support work papered over the aweful long wait
  in atomic context ever since we've added this in
 
  commit b6e45f866465f42b53d803b0c574da0fc508a0e9
  Author: Keith Packard kei...@keithp.com
  Date:   Fri Jan 6 11:34:04 2012 -0800
 
drm/i915: Move reset forcewake processing to gen6_do_reset
 
  Reverting this change should be enough (code moved obviously a bit).
 
  Cheers, Daniel
 
 
  Regards,
  Peter Hurley
 
 
 
  What's the real issue here?
 
 
  That the vblank timestamp needs to be an accurate measurement of a
  realtime event. Sleeping/servicing interrupts while reading
  the registers necessary to compute the timestamp would be bad too.
 
  (edit: which hopefully Mario Kleiner clarified in his reply)
 
  My point earlier was three-fold:
  1. Don't need the preempt_disable() for mainline: all callers are 
  already
holding interrupt-disabling 

Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Skip register reads in i915_get_crtc_scanoutpos()

2013-09-25 Thread Daniel Vetter
On Wed, Sep 25, 2013 at 6:45 AM, Mario Kleiner
mario.klei...@tuebingen.mpg.de wrote:

 Mario, can you please take a look at these patches and ack them? I'd like
 to slurp them in for -next.


 Done in the other e-mails, with some comments. However, i'd like to test
 these a bit and it might take a week or two before i can get my hands on the
 machine with the intel card and the measurement equipment.

 Are we still quite far away from the next merge window?


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


Re: [Intel-gfx] Fwd: linux-next: Tree for Jul 1 [ drm-intel-next: Several call-traces ]

2013-09-25 Thread Daniel Vetter
On Wed, Sep 25, 2013 at 01:17:52PM +1000, Dave Airlie wrote:
 On Mon, Sep 23, 2013 at 12:14 AM, Michael S. Tsirkin m...@redhat.com wrote:
  On Mon, Aug 26, 2013 at 04:05:11PM +0300, Michael S. Tsirkin wrote:
  On Wed, Aug 21, 2013 at 11:22:58AM +0200, Sedat Dilek wrote:
   [ Re: [Intel-gfx] i915 producing warnings with kernel 3.11-rc5 ]
  
   Hi,
  
   saw your posting in [1]... can you try the patches below?
   Not sure if they apply.
   Did you try v3.11-rc6(+)... or drm-intel-nightly?
  
   Regards,
   - Sedat -
  
   [1] 
   http://lists.freedesktop.org/archives/intel-gfx/2013-August/032154.html
 
  Same thing observed with v3.11-rc7.
 
  I still see this with 3.11.
  Following this message, my VGA monitor goes blank and
  shows an error suggesting a wrong resolution or frequency are set.
  Anyone?
 
 Daniel, Jesse?
 
 still ongoing I think.

Yeah, I've dismissed it since the original issue in this thread is
resolved. But it's something else.

Note to bug reporters: Please don't me-too, but start a new thread/report
- almost every gfx bug is different, even if it superficially looks the
same.

Michael, can you please boot with drm.debug=0xe, reproduce the problem and
then attach the complete dmesg? Please make sure dmesg contains everything
since boot-up, if not please increase the dmesg buffer size with
log_buf_len=4M.

Also please test the latest drm-intel-fixes release to check that we
haven't just forgotten to send the patch to stable (there's at least one
flags mismatch fix already in-flight to 3.11 stable kernels).

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


Re: [Intel-gfx] [PATCH] drm/i915: Fix VLV eDP timing

2013-09-25 Thread Daniel Vetter
On Wed, Sep 25, 2013 at 03:36:52PM +0800, Chon Ming Lee wrote:
 Fix the typo in previous commit for DP 1.62 divisor.
 
 drm/i915: Move Valleyview DP DPLL divisor calc to intel_dp_set_clock v2
 
 Reported-by: Jesse Barnes jbar...@virtuousgeek.org 
 Signed-off-by: Chon Ming Lee chon.ming@intel.com

Ville just submitted the same patch so I guess this is right ;-)

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


Re: [Intel-gfx] [PATCH 3/3] drm/i915: Improve the accuracy of get_scanout_pos on CTG+

2013-09-25 Thread Ville Syrjälä
On Wed, Sep 25, 2013 at 05:12:54AM +0200, Mario Kleiner wrote:
 
 
 On 23.09.13 12:02, ville.syrj...@linux.intel.com wrote:
  From: Ville Syrjälä ville.syrj...@linux.intel.com
 
  The DSL register increments at the start of horizontal sync, so it
  manages to miss the entire active portion of the current line.
 
  Improve the get_scanoutpos accuracy a bit when the scanout position is
  close to the start or end of vblank. We can do that by double checking
  the DSL value against the vblank status bit from ISR.
 
  Cc: Mario Kleiner mario.klei...@tuebingen.mpg.de
  Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
  ---
drivers/gpu/drm/i915/i915_irq.c | 53 
  +
1 file changed, 53 insertions(+)
 
  diff --git a/drivers/gpu/drm/i915/i915_irq.c 
  b/drivers/gpu/drm/i915/i915_irq.c
  index 4f74f0c..14b42d9 100644
  --- a/drivers/gpu/drm/i915/i915_irq.c
  +++ b/drivers/gpu/drm/i915/i915_irq.c
  @@ -567,6 +567,47 @@ static u32 gm45_get_vblank_counter(struct drm_device 
  *dev, int pipe)
  return I915_READ(reg);
}
 
  +static bool g4x_pipe_in_vblank(struct drm_device *dev, enum pipe pipe)
  +{
  +   struct drm_i915_private *dev_priv = dev-dev_private;
  +   uint32_t status;
  +
  +   if (IS_VALLEYVIEW(dev)) {
  +   status = pipe == PIPE_A ?
  +   I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT :
  +   I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
  +
  +   return I915_READ(VLV_ISR)  status;
  +   } else if (IS_G4X(dev)) {
  +   status = pipe == PIPE_A ?
  +   I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT :
  +   I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
  +
  +   return I915_READ(ISR)  status;
  +   } else if (INTEL_INFO(dev)-gen  7) {
  +   status = pipe == PIPE_A ?
  +   DE_PIPEA_VBLANK :
  +   DE_PIPEB_VBLANK;
  +
  +   return I915_READ(DEISR)  status;
  +   } else {
  +   switch (pipe) {
  +   default:
  +   case PIPE_A:
  +   status = DE_PIPEA_VBLANK_IVB;
  +   break;
  +   case PIPE_B:
  +   status = DE_PIPEB_VBLANK_IVB;
  +   break;
  +   case PIPE_C:
  +   status = DE_PIPEC_VBLANK_IVB;
  +   break;
  +   }
  +
  +   return I915_READ(DEISR)  status;
  +   }
  +}
  +
static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
   int *vpos, int *hpos)
{
  @@ -616,6 +657,18 @@ static int i915_get_crtc_scanoutpos(struct drm_device 
  *dev, int pipe,
   * scanout position from Display scan line register.
   */
  position = I915_READ(PIPEDSL(pipe))  0x1fff;
  +
  +   /*
  +* The scanline counter increments at the leading edge
  +* of hsync, ie. it completely misses the active portion
  +* of the line. Fix up the counter at both edges of vblank
  +* to get a more accurate picture whether we're in vblank
  +* or not.
  +*/
  +   in_vbl = g4x_pipe_in_vblank(dev, pipe);
  +   if ((in_vbl  position == vbl_start - 1) ||
  +   (!in_vbl  position == vbl_end - 1))
  +   position = (position + 1) % vtotal;
  } else {
  /* Have access to pixelcount since start of frame.
   * We can split this into vertical and horizontal
 
 
 This one i don't know. I think i can't follow the logic, but i don't 
 know enough about the way the intel hw counts.
 
 Do you mean the counter increments when the scanline is over, instead of 
 when it begins?

Let me draw a picture of the scanline (not to scale):

 |X|-|___|---|
  horiz. active   horiz. sync
 ^   ^
 |   |
 first pixel this is where the
 of the line scanline counter increments

 With this correction by +1 at the edges of vblank, the scanlines at 
 vbl_start and vbl_end would be reported twice, for two successive 
 scanline durations, that seems a bit weird and asymmetric to the rest of 
 the scanline positions. Wouldn't it make more sense to simply always add 
 1 for a smaller overall error, given that hblank is shorter than the 
 active scanout part of a scanline?

Since the counter increments too late, drm_handle_vblank()
may get the wrong idea ie. something like this may happen:

1. vblank irq triggered
2. drm_handle_vblank() gets called
3. i915_get_crtc_scanoutpos() returns vbl_start-1 as the scanline
4. delta_ns calculation gets confused and tries to correct for it

Now, the correction you do for delta_ns should handle this, but
I don't like having such kludges in common code, and we can handle
it in the driver as I've demonstrated. But yeah, I suppose it can
make the error slightly less stable.

For some other uses (atomic page flip stuff) of the 

Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Skip register reads in i915_get_crtc_scanoutpos()

2013-09-25 Thread Ville Syrjälä
On Wed, Sep 25, 2013 at 04:35:56AM +0200, Mario Kleiner wrote:
 On 23.09.13 13:48, ville.syrj...@linux.intel.com wrote:
  From: Ville Syrjälä ville.syrj...@linux.intel.com
 
  We have all the information we need in the mode structure, so going and
  reading it from the hardware is pointless, and slower.
 
  We never populated -get_vblank_timestamp() in the UMS case, and as that
  is the only way we'd ever call -get_scanout_position(), we can
  completely ignore UMS in i915_get_crtc_scanoutpos().
 
  Also reorganize intel_irq_init() a bit to clarify the KMS vs. UMS
  situation.
 
  v2: Drop UMS code
 
  Cc: Mario Kleiner mario.klei...@tuebingen.mpg.de
  Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
  ---
drivers/gpu/drm/i915/i915_irq.c | 43 
  -
1 file changed, 17 insertions(+), 26 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/i915_irq.c 
  b/drivers/gpu/drm/i915/i915_irq.c
  index b356dc1..058f099 100644
  --- a/drivers/gpu/drm/i915/i915_irq.c
  +++ b/drivers/gpu/drm/i915/i915_irq.c
  @@ -570,24 +570,29 @@ static u32 gm45_get_vblank_counter(struct drm_device 
  *dev, int pipe)
static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
   int *vpos, int *hpos)
{
  -   drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev-dev_private;
  -   u32 vbl = 0, position = 0;
  +   struct drm_i915_private *dev_priv = dev-dev_private;
  +   struct drm_crtc *crtc = dev_priv-pipe_to_crtc_mapping[pipe];
  +   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
  +   const struct drm_display_mode *mode = intel_crtc-config.adjusted_mode;
  +   u32 position;
  int vbl_start, vbl_end, htotal, vtotal;
  bool in_vbl = true;
  int ret = 0;
  -   enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
  - pipe);
 
  -   if (!i915_pipe_enabled(dev, pipe)) {
  +   if (!intel_crtc-active) {
  DRM_DEBUG_DRIVER(trying to get scanoutpos for disabled 
   pipe %c\n, pipe_name(pipe));
  return 0;
  }
 
  -   /* Get vtotal. */
  -   vtotal = 1 + ((I915_READ(VTOTAL(cpu_transcoder))  16)  0x1fff);
  +   htotal = mode-crtc_htotal;
  +   vtotal = mode-crtc_vtotal;
  +   vbl_start = mode-crtc_vblank_start;
  +   vbl_end = mode-crtc_vblank_end;
 
  -   if (INTEL_INFO(dev)-gen = 4) {
  +   ret |= DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_ACCURATE;
  +
  +   if (IS_G4X(dev) || INTEL_INFO(dev)-gen = 5) {
  /* No obvious pixelcount register. Only query vertical
   * scanout position from Display scan line register.
   */
  @@ -605,29 +610,16 @@ static int i915_get_crtc_scanoutpos(struct drm_device 
  *dev, int pipe,
   */
  position = (I915_READ(PIPEFRAMEPIXEL(pipe))  PIPE_PIXEL_MASK) 
   PIPE_PIXEL_SHIFT;
 
  -   htotal = 1 + ((I915_READ(HTOTAL(cpu_transcoder))  16)  
  0x1fff);
  *vpos = position / htotal;
  *hpos = position - (*vpos * htotal);
  }
 
  -   /* Query vblank area. */
  -   vbl = I915_READ(VBLANK(cpu_transcoder));
  -
  -   /* Test position against vblank region. */
  -   vbl_start = vbl  0x1fff;
  -   vbl_end = (vbl  16)  0x1fff;
  -
  -   if ((*vpos  vbl_start) || (*vpos  vbl_end))
  -   in_vbl = false;
  +   in_vbl = *vpos = vbl_start  *vpos  vbl_end;
 
 I think this should be a = instead of  in *vpos  vbl_end, if it is 
 meant to be equal to the line it replaces (not   is =), unless the 
 original comparison was off-by-one?

Yeah, I think the original was wrong, in more ways than one. It forgot
to add +1 to vbl_start/end, and then it did the comparison wrong as
well.

 
   +  in_vbl = *vpos = vbl_start  *vpos = vbl_end;
 
 Other than that, it looks good.
 
 Reviewed-by: mario.kleiner...@gmail.com
 
 
  /* Inside upper part of vblank area? Apply corrective offset: */
  if (in_vbl  (*vpos = vbl_start))
  *vpos = *vpos - vtotal;
 
  -   /* Readouts valid? */
  -   if (vbl  0)
  -   ret |= DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_ACCURATE;
  -
  /* In vblank? */
  if (in_vbl)
  ret |= DRM_SCANOUTPOS_INVBL;
  @@ -3148,11 +3140,10 @@ void intel_irq_init(struct drm_device *dev)
  dev-driver-get_vblank_counter = gm45_get_vblank_counter;
  }
 
  -   if (drm_core_check_feature(dev, DRIVER_MODESET))
  +   if (drm_core_check_feature(dev, DRIVER_MODESET)) {
  dev-driver-get_vblank_timestamp = i915_get_vblank_timestamp;
  -   else
  -   dev-driver-get_vblank_timestamp = NULL;
  -   dev-driver-get_scanout_position = i915_get_crtc_scanoutpos;
  +   dev-driver-get_scanout_position = i915_get_crtc_scanoutpos;
  +   }
 
  if (IS_VALLEYVIEW(dev)) {
  dev-driver-irq_handler = valleyview_irq_handler;
 

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx 

Re: [Intel-gfx] Fwd: linux-next: Tree for Jul 1 [ drm-intel-next: Several call-traces ]

2013-09-25 Thread Michael S. Tsirkin
On Wed, Sep 25, 2013 at 09:56:52AM +0200, Daniel Vetter wrote:
 On Wed, Sep 25, 2013 at 01:17:52PM +1000, Dave Airlie wrote:
  On Mon, Sep 23, 2013 at 12:14 AM, Michael S. Tsirkin m...@redhat.com 
  wrote:
   On Mon, Aug 26, 2013 at 04:05:11PM +0300, Michael S. Tsirkin wrote:
   On Wed, Aug 21, 2013 at 11:22:58AM +0200, Sedat Dilek wrote:
[ Re: [Intel-gfx] i915 producing warnings with kernel 3.11-rc5 ]
   
Hi,
   
saw your posting in [1]... can you try the patches below?
Not sure if they apply.
Did you try v3.11-rc6(+)... or drm-intel-nightly?
   
Regards,
- Sedat -
   
[1] 
http://lists.freedesktop.org/archives/intel-gfx/2013-August/032154.html
  
   Same thing observed with v3.11-rc7.
  
   I still see this with 3.11.
   Following this message, my VGA monitor goes blank and
   shows an error suggesting a wrong resolution or frequency are set.
   Anyone?
  
  Daniel, Jesse?
  
  still ongoing I think.
 
 Yeah, I've dismissed it since the original issue in this thread is
 resolved. But it's something else.
 
 Note to bug reporters: Please don't me-too, but start a new thread/report
 - almost every gfx bug is different, even if it superficially looks the
 same.
 
 Michael, can you please boot with drm.debug=0xe, reproduce the problem and
 then attach the complete dmesg? Please make sure dmesg contains everything
 since boot-up, if not please increase the dmesg buffer size with
 log_buf_len=4M.
 
 Also please test the latest drm-intel-fixes release to check that we
 haven't just forgotten to send the patch to stable (there's at least one
 flags mismatch fix already in-flight to 3.11 stable kernels).
 
 Thanks, Daniel
 -- 
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch


Will do. Holidays starting now so expect some reports
next week.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Fix VLV eDP timing v2

2013-09-25 Thread Daniel Vetter
On Wed, Sep 25, 2013 at 03:47:51PM +0800, Chon Ming Lee wrote:
 Fix the typo in previous commit for DP 1.62 divisor.
 drm/i915: Move Valleyview DP DPLL divisor calc to intel_dp_set_clock v2
 
 v2: sigh, the m1 div is 3.
 
 Reported-by: Jesse Barnes jbar...@virtuousgeek.org 
 Signed-off-by: Chon Ming Lee chon.ming@intel.com

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


Re: [Intel-gfx] [PATCH v3 2/2] drm/i915/hsw: add flag has_audio in crtc config

2013-09-25 Thread Lin, Mengdong
Hi Daniel,

Thanks for your advice!
Would you help working on this patch? Or can I continue after a few days, I 
have some Android support task these days.

Regards
Mengdong

 -Original Message-
 From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
 Sent: Tuesday, September 24, 2013 8:48 PM
 To: Lin, Mengdong
 Cc: intel-gfx@lists.freedesktop.org
 Subject: Re: [Intel-gfx] [PATCH v3 2/2] drm/i915/hsw: add flag has_audio in 
 crtc
 config
 
 On Tue, Sep 24, 2013 at 01:01:37AM -0400, mengdong@intel.com wrote:
  From: Mengdong Lin mengdong@intel.com
 
  This patch adds a flag has_audio for audio presence in intel_crtc-config.
  HMDI and DP encoders set this flag in their computer_config() if the
  external monitor supports audio. Later audio sequence will check this flag.
 
  Signed-off-by: Mengdong Lin mengdong@intel.com
 
 [snip]
 
  diff --git a/drivers/gpu/drm/i915/intel_dp.c
  b/drivers/gpu/drm/i915/intel_dp.c index 26e162b..4bc125e 100644
  --- a/drivers/gpu/drm/i915/intel_dp.c
  +++ b/drivers/gpu/drm/i915/intel_dp.c
  @@ -764,6 +764,8 @@ found:
 
  intel_dp_set_clock(encoder, pipe_config, intel_dp-link_bw);
 
  +   pipe_config-has_audio = intel_dp-has_audio;
  +
 
 This should only be set when we actually have an audio-capable monitor and
 want to enable the audio output on the port. Furthermore you need to add hw
 state readout support for this boolean to haswell_get_pipe_config and also the
 relevant state check code to intel_pipe_config_compare (by adding
 PIPE_CONF_CHECK_I(has_audio)).
 
 Same applies to the intel_hdmi.c part ofc.
 
 Cheers, Daniel
 
  return true;
   }
 
  diff --git a/drivers/gpu/drm/i915/intel_drv.h
  b/drivers/gpu/drm/i915/intel_drv.h
  index b7d6e09..2bdc23c 100644
  --- a/drivers/gpu/drm/i915/intel_drv.h
  +++ b/drivers/gpu/drm/i915/intel_drv.h
  @@ -290,6 +290,7 @@ struct intel_crtc_config {
  struct intel_link_m_n fdi_m_n;
 
  bool ips_enabled;
  +   bool has_audio;
   };
 
   struct intel_crtc {
  @@ -303,7 +304,6 @@ struct intel_crtc {
   * some outputs connected to this crtc.
   */
  bool active;
  -   bool eld_vld;
  bool primary_disabled; /* is the crtc obscured by a plane? */
  bool lowfreq_avail;
  struct intel_overlay *overlay;
  diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
  b/drivers/gpu/drm/i915/intel_hdmi.c
  index 2fd3fd5..09c9d69 100644
  --- a/drivers/gpu/drm/i915/intel_hdmi.c
  +++ b/drivers/gpu/drm/i915/intel_hdmi.c
  @@ -864,6 +864,8 @@ bool intel_hdmi_compute_config(struct
 intel_encoder *encoder,
  return false;
  }
 
  +   pipe_config-has_audio = intel_hdmi-has_audio;
  +
  return true;
   }
 
  --
  1.8.1.2
 
  ___
  Intel-gfx mailing list
  Intel-gfx@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/intel-gfx
 
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v3 1/2] drm/i915/hsw: Add display Audio codec disable sequence for Haswell

2013-09-25 Thread Lin, Mengdong
Hi,

Is it okay to integrate this patch at first?

We can check whether vblank wait can be removed safely later

Thanks
Mengdong

 -Original Message-
 From: Lin, Mengdong
 Sent: Tuesday, September 24, 2013 1:01 PM
 To: intel-gfx@lists.freedesktop.org
 Cc: Arora, MukeshX; Lin, Mengdong
 Subject: [PATCH v3 1/2] drm/i915/hsw: Add display Audio codec disable
 sequence for Haswell
 
 From: Mukesh mukeshx.ar...@intel.com
 
 The code defines a new function intel_disable_audio() to implement the entire
 audio codec disable sequence, called by intel_disable_ddi() in DDI port
 disablement. This audio codec disbale sequence is implemented as per the
 recommendation of the Bspec.
 
 [Patch modified by Mendong to apply to both HDMI and DP port.]
 
 Signed-off-by: Mukesh Arora mukeshx.ar...@intel.com
 Signed-off-by: Mengdong Lin mengdong@intel.com
 Reviewed-by: Ben Widawsky b...@bwidawsk.net
 
 diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
 b/drivers/gpu/drm/i915/intel_ddi.c
 index b042ee5..bda9181 100644
 --- a/drivers/gpu/drm/i915/intel_ddi.c
 +++ b/drivers/gpu/drm/i915/intel_ddi.c
 @@ -1088,6 +1088,50 @@ static void intel_ddi_post_disable(struct
 intel_encoder *intel_encoder)
   I915_WRITE(PORT_CLK_SEL(port), PORT_CLK_SEL_NONE);  }
 
 +/* audio codec disable sequence, as per Bspec */ void
 +intel_disable_audio(struct intel_encoder *intel_encoder) {
 + int type = intel_encoder-type;
 + struct drm_encoder *encoder = intel_encoder-base;
 + struct drm_device *dev = encoder-dev;
 + struct drm_i915_private *dev_priv = dev-dev_private;
 + struct intel_crtc *intel_crtc = to_intel_crtc(encoder-crtc);
 + int pipe = intel_crtc-pipe;
 + int aud_config = HSW_AUD_CFG(pipe);
 + u32 temp;
 +
 + /* need not disable sample fabrication because never enabled */
 +
 + /* disable timestamps */
 + temp = I915_READ(aud_config);
 + if (type == INTEL_OUTPUT_HDMI)
 + temp = ~AUD_CONFIG_N_VALUE_INDEX;
 + else if (type == INTEL_OUTPUT_DISPLAYPORT)
 + temp |= AUD_CONFIG_N_VALUE_INDEX;
 + else
 + return;
 + temp |= AUD_CONFIG_N_PROG_ENABLE;
 + temp =
 ~(AUD_CONFIG_UPPER_N_VALUE|AUD_CONFIG_LOWER_N_VALUE);
 + I915_WRITE(aud_config, temp);
 + POSTING_READ(aud_config);
 +
 + /* Disable ELDV and ELD buffer */
 + temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
 + temp = ~(AUDIO_ELD_VALID_A  (pipe * 4));
 + I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp);
 + POSTING_READ(HSW_AUD_PIN_ELD_CP_VLD);
 +
 + /* Wait for 2 vertical blanks */
 + intel_wait_for_vblank(dev, pipe);
 + intel_wait_for_vblank(dev, pipe);
 +
 + /* Disable audio PD. This is optional as per Bspec.  */
 + temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
 + temp = ~(AUDIO_OUTPUT_ENABLE_A  (pipe * 4));
 + I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp);
 + POSTING_READ(HSW_AUD_PIN_ELD_CP_VLD);
 +}
 +
  static void intel_enable_ddi(struct intel_encoder *intel_encoder)  {
   struct drm_encoder *encoder = intel_encoder-base; @@ -1132,18
 +1176,10 @@ static void intel_disable_ddi(struct intel_encoder
 *intel_encoder)
   struct drm_encoder *encoder = intel_encoder-base;
   struct drm_crtc *crtc = encoder-crtc;
   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 - int pipe = intel_crtc-pipe;
   int type = intel_encoder-type;
 - struct drm_device *dev = encoder-dev;
 - struct drm_i915_private *dev_priv = dev-dev_private;
 - uint32_t tmp;
 
 - if (intel_crtc-eld_vld  type != INTEL_OUTPUT_EDP) {
 - tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
 - tmp = ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A) 
 -  (pipe * 4));
 - I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
 - }
 + if (intel_crtc-eld_vld  type != INTEL_OUTPUT_EDP)
 + intel_disable_audio(intel_encoder);
 
   if (type == INTEL_OUTPUT_EDP) {
   struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 --
 1.8.1.2

___
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: Improve the accuracy of get_scanout_pos on CTG+

2013-09-25 Thread Daniel Vetter
On Wed, Sep 25, 2013 at 11:11:30AM +0300, Ville Syrjälä wrote:
 On Wed, Sep 25, 2013 at 05:12:54AM +0200, Mario Kleiner wrote:
  This one i don't know. I think i can't follow the logic, but i don't 
  know enough about the way the intel hw counts.
  
  Do you mean the counter increments when the scanline is over, instead of 
  when it begins?
 
 Let me draw a picture of the scanline (not to scale):
 
  |X|-|___|---|
   horiz. active   horiz. sync
  ^   ^
  |   |
  first pixel this is where the
  of the line scanline counter increments
 
  With this correction by +1 at the edges of vblank, the scanlines at 
  vbl_start and vbl_end would be reported twice, for two successive 
  scanline durations, that seems a bit weird and asymmetric to the rest of 
  the scanline positions. Wouldn't it make more sense to simply always add 
  1 for a smaller overall error, given that hblank is shorter than the 
  active scanout part of a scanline?
 
 Since the counter increments too late, drm_handle_vblank()
 may get the wrong idea ie. something like this may happen:
 
 1. vblank irq triggered
 2. drm_handle_vblank() gets called
 3. i915_get_crtc_scanoutpos() returns vbl_start-1 as the scanline
 4. delta_ns calculation gets confused and tries to correct for it
 
 Now, the correction you do for delta_ns should handle this, but
 I don't like having such kludges in common code, and we can handle
 it in the driver as I've demonstrated. But yeah, I suppose it can
 make the error slightly less stable.
 
 For some other uses (atomic page flip stuff) of the scanline position,
 I definitely want this correction since I need accurate information
 whether the position has passed vblank start.

At least on modern platforms I think we should take a good look at the
timestamp registers. With those the only thing we essentially need to do
is to correct the spot where the timestamp is taken to make it suitable
for OML_sync ... But doing such a switch is something for future work ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v3 0/4] Fix Win8 backlight issue

2013-09-25 Thread Aaron Lu
On Wed, Sep 25, 2013 at 10:29:37AM +0200, Jörg Otte wrote:
 Backlight can't be modified with this patch set -  neither with function
 keys nor
 with the gui. It is a step backward to v3.11-rc1

Thanks for the test.

Please check /sys/class/backlight, is there only one link file
intel_backlight? If so, please cd inside and try modify the brightness
file using echo with some values in the range of 0 - max_brightness,
does the backlight level change when you echo a new value? I guess it
doesn't, or you wouldn't notice problem. If indeed so, perhaps file a
bug at http://bugzilla.kernel.org, Drivers/Video(DRI-Intel)? I suppose
Jani and Daniel can help fix your problem.

Thanks,
Aaron

 
 Video driver: i915
 FUJITSU LIFEBOOK AH532/FJNBB1C, BIOS Version 1.09 05/22/2012
 
 acpi_backlight=video works.
 
 Jörg
 
 
 2013/9/24 Igor Gnatenko i.gnatenko.br...@gmail.com
 
  On Tue, 2013-09-24 at 17:47 +0800, Aaron Lu wrote:
   v3:
   1 Add a new patch 4/4 to fix some problems in thinkpad-acpi module;
   2 Remove unnecessary function acpi_video_unregister introduced in
 patch 2/3 as pointed out by Jani Nikula.
  
   v2:
   v1 has the subject of Rework ACPI video driver and is posted here:
   http://lkml.org/lkml/2013/9/9/74
   Since the objective is really to fix Win8 backlight issues, I changed
   the subject in this version, sorry about that.
  
   This patchset has three patches, the first introduced a new API named
   backlight_device_registered in backlight layer that can be used for
   backlight interface provider module to check if a specific type backlight
   interface has been registered, see changelog for patch 1/3 for details.
   Then patch 2/3 does the cleanup to sepeate the backlight control and
   event delivery functionality in the ACPI video module and patch 3/3
   solves some Win8 backlight control problems by avoiding register ACPI
   video's backlight interface if:
   1 Kernel cmdline option acpi_backlight=video is not given;
   2 This is a Win8 system;
   3 Native backlight control interface exists.
  
   Technically, patch 2/3 is not required to fix the issue here. So if you
   think it is not necessary, I can remove it from the series.
  
   Aaron Lu (4):
 backlight: introduce backlight_device_registered
 ACPI / video: seperate backlight control and event interface
 ACPI / video: Do not register backlight if win8 and native interface
   exists
 thinkpad-acpi: fix handle locate for video and query of _BCL
  
drivers/acpi/internal.h  |   5 +-
drivers/acpi/video.c | 442
  ---
drivers/acpi/video_detect.c  |  14 +-
drivers/platform/x86/thinkpad_acpi.c |  31 ++-
drivers/video/backlight/backlight.c  |  31 +++
include/acpi/video.h |   2 +
include/linux/backlight.h|   4 +
7 files changed, 324 insertions(+), 205 deletions(-)
  
 
  Excellent! I've tested this patch-set on ThinkPad X230 and backlight
  issue is exhausted.
 
  Thanks.
 
  --
  Igor Gnatenko
  Fedora release 20 (Heisenbug)
  Linux 3.11.1-300.fc20.x86_64
 
 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 09/13] drm/i915: Add a tracepoint for using a semaphore

2013-09-25 Thread Ville Syrjälä
On Mon, Sep 23, 2013 at 05:33:26PM -0300, Rodrigo Vivi wrote:
 From: Chris Wilson ch...@chris-wilson.co.uk
 
 So that we can find the callers who introduce a ring stall. A single
 ring stall is not too unwelcome, the right issue becomes when they start
 to interlock and prevent any concurrent work. That, however, is a little
 tricker to detect with a mere tracepoint!
 
 v2: Rebrand it as a ring event, rather than an object event.
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com

Just wondering if we would want to see the seqno(s) in the trace as well?

But anyway, the patch looks fine.
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

 ---
  drivers/gpu/drm/i915/i915_gem.c   |  2 ++
  drivers/gpu/drm/i915/i915_trace.h | 19 +++
  2 files changed, 21 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
 index d68cc5c..4a16491 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -2614,6 +2614,8 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
   if (ret)
   return ret;
  
 + trace_i915_gem_ring_sync_to(from, to);
 +
   ret = to-sync_to(to, from, seqno);
   if (!ret)
   /* We use last_read_seqno because sync_to()
 diff --git a/drivers/gpu/drm/i915/i915_trace.h 
 b/drivers/gpu/drm/i915/i915_trace.h
 index 5c8e36a..48e8f07 100644
 --- a/drivers/gpu/drm/i915/i915_trace.h
 +++ b/drivers/gpu/drm/i915/i915_trace.h
 @@ -233,6 +233,25 @@ TRACE_EVENT(i915_gem_evict_everything,
   TP_printk(dev=%d, __entry-dev)
  );
  
 +TRACE_EVENT(i915_gem_ring_sync_to,
 + TP_PROTO(struct intel_ring_buffer *from, struct intel_ring_buffer 
 *to),
 + TP_ARGS(from, to),
 +
 + TP_STRUCT__entry(
 +  __field(u32, dev)
 +  __field(u32, sync_from)
 +  __field(u32, sync_to)
 +  ),
 +
 + TP_fast_assign(
 +__entry-dev = from-dev-primary-index;
 +__entry-sync_from = from-id;
 +__entry-sync_to = to-id;
 +),
 +
 + TP_printk(dev=%u, sync-from=%u, sync-to=%u, __entry-dev, 
 __entry-sync_from, __entry-sync_to)
 +);
 +
  TRACE_EVENT(i915_gem_ring_dispatch,
   TP_PROTO(struct intel_ring_buffer *ring, u32 flags),
   TP_ARGS(ring, flags),
 -- 
 1.8.1.4
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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


[Intel-gfx] [PATCH] drm/i915: Show WT caching in debugfs

2013-09-25 Thread Chris Wilson
Add the missing cache-level to the describe_obj() function for debug and
error reporting.

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

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 28c886d..e8ffd57 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -324,7 +324,7 @@ struct drm_i915_error_state {
u32 dirty:1;
u32 purgeable:1;
s32 ring:4;
-   u32 cache_level:2;
+   u32 cache_level:3;
} **active_bo, **pinned_bo;
u32 *active_bo_count, *pinned_bo_count;
struct intel_overlay_error_state *overlay;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
b/drivers/gpu/drm/i915/i915_gpu_error.c
index c3ff6bd..0a49b65 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1012,6 +1012,7 @@ const char *i915_cache_level_str(int type)
case I915_CACHE_NONE: return  uncached;
case I915_CACHE_LLC: return  snooped or LLC;
case I915_CACHE_L3_LLC: return  L3+LLC;
+   case I915_CACHE_WT: return  WT;
default: return ;
}
 }
-- 
1.8.4.rc3

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


Re: [Intel-gfx] [PATCH 09/13] drm/i915: Add a tracepoint for using a semaphore

2013-09-25 Thread Daniel Vetter
On Wed, Sep 25, 2013 at 12:34:37PM +0300, Ville Syrjälä wrote:
 On Mon, Sep 23, 2013 at 05:33:26PM -0300, Rodrigo Vivi wrote:
  From: Chris Wilson ch...@chris-wilson.co.uk
  
  So that we can find the callers who introduce a ring stall. A single
  ring stall is not too unwelcome, the right issue becomes when they start
  to interlock and prevent any concurrent work. That, however, is a little
  tricker to detect with a mere tracepoint!
  
  v2: Rebrand it as a ring event, rather than an object event.
  
  Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
  Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
 
 Just wondering if we would want to see the seqno(s) in the trace as well?

Hm yeah, I guess the seqno we're syncing on the from ring would be useful
to gauge how much parallelism there really is. Chris, care to respin?
-Daniel

 
 But anyway, the patch looks fine.
 Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
 
  ---
   drivers/gpu/drm/i915/i915_gem.c   |  2 ++
   drivers/gpu/drm/i915/i915_trace.h | 19 +++
   2 files changed, 21 insertions(+)
  
  diff --git a/drivers/gpu/drm/i915/i915_gem.c 
  b/drivers/gpu/drm/i915/i915_gem.c
  index d68cc5c..4a16491 100644
  --- a/drivers/gpu/drm/i915/i915_gem.c
  +++ b/drivers/gpu/drm/i915/i915_gem.c
  @@ -2614,6 +2614,8 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
  if (ret)
  return ret;
   
  +   trace_i915_gem_ring_sync_to(from, to);
  +
  ret = to-sync_to(to, from, seqno);
  if (!ret)
  /* We use last_read_seqno because sync_to()
  diff --git a/drivers/gpu/drm/i915/i915_trace.h 
  b/drivers/gpu/drm/i915/i915_trace.h
  index 5c8e36a..48e8f07 100644
  --- a/drivers/gpu/drm/i915/i915_trace.h
  +++ b/drivers/gpu/drm/i915/i915_trace.h
  @@ -233,6 +233,25 @@ TRACE_EVENT(i915_gem_evict_everything,
  TP_printk(dev=%d, __entry-dev)
   );
   
  +TRACE_EVENT(i915_gem_ring_sync_to,
  +   TP_PROTO(struct intel_ring_buffer *from, struct intel_ring_buffer 
  *to),
  +   TP_ARGS(from, to),
  +
  +   TP_STRUCT__entry(
  +__field(u32, dev)
  +__field(u32, sync_from)
  +__field(u32, sync_to)
  +),
  +
  +   TP_fast_assign(
  +  __entry-dev = from-dev-primary-index;
  +  __entry-sync_from = from-id;
  +  __entry-sync_to = to-id;
  +  ),
  +
  +   TP_printk(dev=%u, sync-from=%u, sync-to=%u, __entry-dev, 
  __entry-sync_from, __entry-sync_to)
  +);
  +
   TRACE_EVENT(i915_gem_ring_dispatch,
  TP_PROTO(struct intel_ring_buffer *ring, u32 flags),
  TP_ARGS(ring, flags),
  -- 
  1.8.1.4
  
  ___
  Intel-gfx mailing list
  Intel-gfx@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/intel-gfx
 
 -- 
 Ville Syrjälä
 Intel OTC
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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


Re: [Intel-gfx] [PATCH] drm/i915: Show WT caching in debugfs

2013-09-25 Thread Daniel Vetter
On Wed, Sep 25, 2013 at 10:23:19AM +0100, Chris Wilson wrote:
 Add the missing cache-level to the describe_obj() function for debug and
 error reporting.
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk

Merged to dinq ...

 ---
  drivers/gpu/drm/i915/i915_drv.h   | 2 +-
  drivers/gpu/drm/i915/i915_gpu_error.c | 1 +
  2 files changed, 2 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 28c886d..e8ffd57 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -324,7 +324,7 @@ struct drm_i915_error_state {
   u32 dirty:1;
   u32 purgeable:1;
   s32 ring:4;
 - u32 cache_level:2;
 + u32 cache_level:3;

... but I wonder a bit whether we should just use the i915_cache_level
enum here? Same for the ring id maybe. Otoh meh ;-)
-Daniel

   } **active_bo, **pinned_bo;
   u32 *active_bo_count, *pinned_bo_count;
   struct intel_overlay_error_state *overlay;
 diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
 b/drivers/gpu/drm/i915/i915_gpu_error.c
 index c3ff6bd..0a49b65 100644
 --- a/drivers/gpu/drm/i915/i915_gpu_error.c
 +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
 @@ -1012,6 +1012,7 @@ const char *i915_cache_level_str(int type)
   case I915_CACHE_NONE: return  uncached;
   case I915_CACHE_LLC: return  snooped or LLC;
   case I915_CACHE_L3_LLC: return  L3+LLC;
 + case I915_CACHE_WT: return  WT;
   default: return ;
   }
  }
 -- 
 1.8.4.rc3
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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


Re: [Intel-gfx] [PATCH] drm/i915: Show WT caching in debugfs

2013-09-25 Thread Chris Wilson
On Wed, Sep 25, 2013 at 12:14:44PM +0200, Daniel Vetter wrote:
 On Wed, Sep 25, 2013 at 10:23:19AM +0100, Chris Wilson wrote:
  Add the missing cache-level to the describe_obj() function for debug and
  error reporting.
  
  Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 
 Merged to dinq ...
 
  ---
   drivers/gpu/drm/i915/i915_drv.h   | 2 +-
   drivers/gpu/drm/i915/i915_gpu_error.c | 1 +
   2 files changed, 2 insertions(+), 1 deletion(-)
  
  diff --git a/drivers/gpu/drm/i915/i915_drv.h 
  b/drivers/gpu/drm/i915/i915_drv.h
  index 28c886d..e8ffd57 100644
  --- a/drivers/gpu/drm/i915/i915_drv.h
  +++ b/drivers/gpu/drm/i915/i915_drv.h
  @@ -324,7 +324,7 @@ struct drm_i915_error_state {
  u32 dirty:1;
  u32 purgeable:1;
  s32 ring:4;
  -   u32 cache_level:2;
  +   u32 cache_level:3;
 
 ... but I wonder a bit whether we should just use the i915_cache_level
 enum here? Same for the ring id maybe. Otoh meh ;-)

Previous bytes semi-permanently allocated... A pittance compared to the
overallocated of a libva batchbuffer, but still...
-Chris

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


Re: [Intel-gfx] [PATCH v3 0/4] Fix Win8 backlight issue

2013-09-25 Thread Jani Nikula
On Wed, 25 Sep 2013, Aaron Lu aaron...@intel.com wrote:
 On Wed, Sep 25, 2013 at 10:29:37AM +0200, Jörg Otte wrote:
 Backlight can't be modified with this patch set - neither with
 function keys nor with the gui. It is a step backward to v3.11-rc1

So both hotkeys and GUI work in v3.11-rc1? And v3.12-rc2?

 Thanks for the test.

 Please check /sys/class/backlight, is there only one link file
 intel_backlight?

Indeed, are there others? fujitsu-laptop perhaps? If yes, try
CONFIG_FUJITSU_LAPTOP=n or an appropriate module blacklist.

Just checking, you do have CONFIG_BACKLIGHT_CLASS_DEVICE=y?
Embarrassingly there was a bug in i915 fixed just recently where the
backlight device wasn't registered for
CONFIG_BACKLIGHT_CLASS_DEVICE=m...

 If so, please cd inside and try modify the brightness file using echo
 with some values in the range of 0 - max_brightness, does the
 backlight level change when you echo a new value? I guess it doesn't,
 or you wouldn't notice problem. If indeed so, perhaps file a bug at
 http://bugzilla.kernel.org, Drivers/Video(DRI-Intel)? I suppose Jani
 and Daniel can help fix your problem.

Yup, please check the above, and report back.

BR,
Jani.



 Thanks,
 Aaron

 
 Video driver: i915
 FUJITSU LIFEBOOK AH532/FJNBB1C, BIOS Version 1.09 05/22/2012
 
 acpi_backlight=video works.
 
 Jörg
 
 
 2013/9/24 Igor Gnatenko i.gnatenko.br...@gmail.com
 
  On Tue, 2013-09-24 at 17:47 +0800, Aaron Lu wrote:
   v3:
   1 Add a new patch 4/4 to fix some problems in thinkpad-acpi module;
   2 Remove unnecessary function acpi_video_unregister introduced in
 patch 2/3 as pointed out by Jani Nikula.
  
   v2:
   v1 has the subject of Rework ACPI video driver and is posted here:
   http://lkml.org/lkml/2013/9/9/74
   Since the objective is really to fix Win8 backlight issues, I changed
   the subject in this version, sorry about that.
  
   This patchset has three patches, the first introduced a new API named
   backlight_device_registered in backlight layer that can be used for
   backlight interface provider module to check if a specific type backlight
   interface has been registered, see changelog for patch 1/3 for details.
   Then patch 2/3 does the cleanup to sepeate the backlight control and
   event delivery functionality in the ACPI video module and patch 3/3
   solves some Win8 backlight control problems by avoiding register ACPI
   video's backlight interface if:
   1 Kernel cmdline option acpi_backlight=video is not given;
   2 This is a Win8 system;
   3 Native backlight control interface exists.
  
   Technically, patch 2/3 is not required to fix the issue here. So if you
   think it is not necessary, I can remove it from the series.
  
   Aaron Lu (4):
 backlight: introduce backlight_device_registered
 ACPI / video: seperate backlight control and event interface
 ACPI / video: Do not register backlight if win8 and native interface
   exists
 thinkpad-acpi: fix handle locate for video and query of _BCL
  
drivers/acpi/internal.h  |   5 +-
drivers/acpi/video.c | 442
  ---
drivers/acpi/video_detect.c  |  14 +-
drivers/platform/x86/thinkpad_acpi.c |  31 ++-
drivers/video/backlight/backlight.c  |  31 +++
include/acpi/video.h |   2 +
include/linux/backlight.h|   4 +
7 files changed, 324 insertions(+), 205 deletions(-)
  
 
  Excellent! I've tested this patch-set on ThinkPad X230 and backlight
  issue is exhausted.
 
  Thanks.
 
  --
  Igor Gnatenko
  Fedora release 20 (Heisenbug)
  Linux 3.11.1-300.fc20.x86_64
 
 

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


[Intel-gfx] [PATCH] drm/i915: Add a tracepoint for using a semaphore

2013-09-25 Thread Chris Wilson
So that we can find the callers who introduce a ring stall. A single
ring stall is not too unwelcome, the right issue becomes when they start
to interlock and prevent any concurrent work. That, however, is a little
tricker to detect with a mere tracepoint!

v2: Rebrand it as a ring event, rather than an object event.
v3: Include the seqno in the tracepoint for posterity or something.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_gem.c   |  1 +
 drivers/gpu/drm/i915/i915_trace.h | 26 ++
 2 files changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 82bc029..fa3b373 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2625,6 +2625,7 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
if (ret)
return ret;
 
+   trace_i915_gem_ring_sync_to(from, to, seqno);
ret = to-sync_to(to, from, seqno);
if (!ret)
/* We use last_read_seqno because sync_to()
diff --git a/drivers/gpu/drm/i915/i915_trace.h 
b/drivers/gpu/drm/i915/i915_trace.h
index daa6fdf..6e580c9 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -248,6 +248,32 @@ TRACE_EVENT(i915_gem_evict_vm,
TP_printk(dev=%d, vm=%p, __entry-vm-dev-primary-index, 
__entry-vm)
 );
 
+TRACE_EVENT(i915_gem_ring_sync_to,
+   TP_PROTO(struct intel_ring_buffer *from,
+struct intel_ring_buffer *to,
+u32 seqno),
+   TP_ARGS(from, to, seqno),
+
+   TP_STRUCT__entry(
+__field(u32, dev)
+__field(u32, sync_from)
+__field(u32, sync_to)
+__field(u32, seqno)
+),
+
+   TP_fast_assign(
+  __entry-dev = from-dev-primary-index;
+  __entry-sync_from = from-id;
+  __entry-sync_to = to-id;
+  __entry-seqno = seqno;
+  ),
+
+   TP_printk(dev=%u, sync-from=%u, sync-to=%u, seqno=%u,
+ __entry-dev,
+ __entry-sync_from, __entry-sync_to,
+ __entry-seqno)
+);
+
 TRACE_EVENT(i915_gem_ring_dispatch,
TP_PROTO(struct intel_ring_buffer *ring, u32 seqno, u32 flags),
TP_ARGS(ring, seqno, flags),
-- 
1.8.4.rc3

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


Re: [Intel-gfx] [PATCH] drm/i915: Add a tracepoint for using a semaphore

2013-09-25 Thread Ville Syrjälä
On Wed, Sep 25, 2013 at 11:43:28AM +0100, Chris Wilson wrote:
 So that we can find the callers who introduce a ring stall. A single
 ring stall is not too unwelcome, the right issue becomes when they start
 to interlock and prevent any concurrent work. That, however, is a little
 tricker to detect with a mere tracepoint!
 
 v2: Rebrand it as a ring event, rather than an object event.
 v3: Include the seqno in the tracepoint for posterity or something.
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 ---
  drivers/gpu/drm/i915/i915_gem.c   |  1 +
  drivers/gpu/drm/i915/i915_trace.h | 26 ++
  2 files changed, 27 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
 index 82bc029..fa3b373 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -2625,6 +2625,7 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
   if (ret)
   return ret;
  
 + trace_i915_gem_ring_sync_to(from, to, seqno);
   ret = to-sync_to(to, from, seqno);

Passing the rings in the same order to both might avoid some confusion,
but I don't care enough to withhold my r-b so:
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

   if (!ret)
   /* We use last_read_seqno because sync_to()
 diff --git a/drivers/gpu/drm/i915/i915_trace.h 
 b/drivers/gpu/drm/i915/i915_trace.h
 index daa6fdf..6e580c9 100644
 --- a/drivers/gpu/drm/i915/i915_trace.h
 +++ b/drivers/gpu/drm/i915/i915_trace.h
 @@ -248,6 +248,32 @@ TRACE_EVENT(i915_gem_evict_vm,
   TP_printk(dev=%d, vm=%p, __entry-vm-dev-primary-index, 
 __entry-vm)
  );
  
 +TRACE_EVENT(i915_gem_ring_sync_to,
 + TP_PROTO(struct intel_ring_buffer *from,
 +  struct intel_ring_buffer *to,
 +  u32 seqno),
 + TP_ARGS(from, to, seqno),
 +
 + TP_STRUCT__entry(
 +  __field(u32, dev)
 +  __field(u32, sync_from)
 +  __field(u32, sync_to)
 +  __field(u32, seqno)
 +  ),
 +
 + TP_fast_assign(
 +__entry-dev = from-dev-primary-index;
 +__entry-sync_from = from-id;
 +__entry-sync_to = to-id;
 +__entry-seqno = seqno;
 +),
 +
 + TP_printk(dev=%u, sync-from=%u, sync-to=%u, seqno=%u,
 +   __entry-dev,
 +   __entry-sync_from, __entry-sync_to,
 +   __entry-seqno)
 +);
 +
  TRACE_EVENT(i915_gem_ring_dispatch,
   TP_PROTO(struct intel_ring_buffer *ring, u32 seqno, u32 flags),
   TP_ARGS(ring, seqno, flags),
 -- 
 1.8.4.rc3
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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


Re: [Intel-gfx] [PATCH] drm/i915: Add a tracepoint for using a semaphore

2013-09-25 Thread Daniel Vetter
On Wed, Sep 25, 2013 at 02:29:34PM +0300, Ville Syrjälä wrote:
 On Wed, Sep 25, 2013 at 11:43:28AM +0100, Chris Wilson wrote:
  So that we can find the callers who introduce a ring stall. A single
  ring stall is not too unwelcome, the right issue becomes when they start
  to interlock and prevent any concurrent work. That, however, is a little
  tricker to detect with a mere tracepoint!
  
  v2: Rebrand it as a ring event, rather than an object event.
  v3: Include the seqno in the tracepoint for posterity or something.
  
  Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
  ---
   drivers/gpu/drm/i915/i915_gem.c   |  1 +
   drivers/gpu/drm/i915/i915_trace.h | 26 ++
   2 files changed, 27 insertions(+)
  
  diff --git a/drivers/gpu/drm/i915/i915_gem.c 
  b/drivers/gpu/drm/i915/i915_gem.c
  index 82bc029..fa3b373 100644
  --- a/drivers/gpu/drm/i915/i915_gem.c
  +++ b/drivers/gpu/drm/i915/i915_gem.c
  @@ -2625,6 +2625,7 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
  if (ret)
  return ret;
   
  +   trace_i915_gem_ring_sync_to(from, to, seqno);
  ret = to-sync_to(to, from, seqno);
 
 Passing the rings in the same order to both might avoid some confusion,
 but I don't care enough to withhold my r-b so:
 Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

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


Re: [Intel-gfx] [PATCH 5/6] drm/i915: Use the new vm [un]bind functions

2013-09-25 Thread Daniel Vetter
On Tue, Sep 24, 2013 at 09:58:00AM -0700, Ben Widawsky wrote:
 From: Ben Widawsky b...@bwidawsk.net
 
 Building on the last patch which created the new function pointers in
 the VM for bind/unbind, here we actually put those new function pointers
 to use.
 
 Split out as a separate patch to aid in review. I'm fine with squashing
 into the previous patch if people request it.
 
 v2: Updated to address the smart ggtt which can do aliasing as needed
 Make sure we bind to global gtt when mappable and fenceable. I thought
 we could get away without this initialy, but we cannot.
 
 v3: Make the global GTT binding explicitly use the ggtt VM for
 bind_vma(). While at it, use the new ggtt_vma helper (Chris)
 
 v4: Make the code support the secure dispatch flag, which requires
 special handling during execbuf. This was fixed (incorrectly) later in
 the series, but having it here earlier in the series should be perfectly
 acceptable. (Chris)
 Move do_switch over to the new, special ggtt_vma interface.
 
 v5: Don't use a local variable (or assertion) when setting the batch
 object to the global GTT during secure dispatch (Chris)
 
 v6: Caclulate the exec offset for the secure case (Bug fix missed on
 v4). (Chris)
 Remove redundant check for has_global_gtt_mapping, since it is done in
 bind_vma.
 
 v7: Remove now unused dev_priv in do_switch
 Don't pass the vm to ggtt_offset (error from v6 which I should have
 caught before sending).
 
 v8: Assert, and rework the SNB workaround (to make it a bit clearer)
 code to make sure the VM can't be anything but the GGTT.
 
 v9: Fixing more bugs which can't exist yet on the behest of Chris. Make
 sure that the batch object is properly bound, and added to the global
 VM's active list - for when we use non-global VMs. (Chris) Note that
 there is an ongoing confusion about what bugs existed, but the known
 bugs are fixed here.
 
 v10: Nitpicks on whitespacing etc. introduced in v9 (Chris)
 
 Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
 Signed-off-by: Ben Widawsky b...@bwidawsk.net

Big rant here on two topics:

First splitting a patch into a write new functions, but leave them as
dead code (the previous patch) and kill old code, use new functions
(this patch) is imo pointless. It hampers reviewing (since you can't
compare old and new logic easily because it's split into two patches) and
it adds zero value for bisecting: Compile-testing dead code isn't a useful
additional checkpoint.

Check out Ville's vlv dpll refactoring for a great split-up patch series.

Second is the death of the -insert_entries/-clear_range vfuncs. We
_need_ them in the internal tree and you really can't just kill them. If
you want to, you need to follow three steps:

1. Create new interfaces in the public tree, use the on public platforms
but keeep the old interfaces working.

2. Convert the -internal platforms over.

3. Remove old interface.

Doing 3. before 2. is bonghits and will result in the internal tree being
broken a bit in-between. Since I'm supposed to maintain that I'll undo the
damage here to be able to do a rebase.

Cheers, Daniel

 ---
  drivers/gpu/drm/i915/i915_drv.h|  9 -
  drivers/gpu/drm/i915/i915_gem.c| 33 +++-
  drivers/gpu/drm/i915/i915_gem_context.c|  6 ++-
  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 61 
 --
  drivers/gpu/drm/i915/i915_gem_gtt.c| 48 ++-
  5 files changed, 62 insertions(+), 95 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 9995cdb..e8ae8fd 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -2102,17 +2102,8 @@ int i915_gem_context_destroy_ioctl(struct drm_device 
 *dev, void *data,
  
  /* i915_gem_gtt.c */
  void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev);
 -void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
 - struct drm_i915_gem_object *obj,
 - enum i915_cache_level cache_level);
 -void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt,
 -   struct drm_i915_gem_object *obj);
 -
  void i915_gem_restore_gtt_mappings(struct drm_device *dev);
  int __must_check i915_gem_gtt_prepare_object(struct drm_i915_gem_object 
 *obj);
 -void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
 - enum i915_cache_level cache_level);
 -void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj);
  void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj);
  void i915_gem_init_global_gtt(struct drm_device *dev);
  void i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long start,
 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
 index f6c8b0e..378d4ef 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -2693,12 +2693,8 @@ int i915_vma_unbind(struct i915_vma *vma)
  
   

[Intel-gfx] [PATCH] drm/i915: Fix up usage of SHRINK_STOP

2013-09-25 Thread Daniel Vetter
In

commit 81e49f811404f428a9d9a63295a0c267e802fa12
Author: Glauber Costa glom...@openvz.org
Date:   Wed Aug 28 10:18:13 2013 +1000

i915: bail out earlier when shrinker cannot acquire mutex

SHRINK_STOP was added to tell the core shrinker code to bail out and
go to the next shrinker since the i915 shrinker couldn't acquire
required locks. But the SHRINK_STOP return code was added to the
-count_objects callback and not the -scan_objects callback as it
should have been, resulting in tons of dmesg noise like

shrink_slab: i915_gem_inactive_scan+0x0/0x9c negative objects to delete 
nr=-x

Fix discusssed with Dave Chinner.

References: http://www.spinics.net/lists/intel-gfx/msg33597.html
Reported-by: Knut Petersen knut_peter...@t-online.de
Cc: Knut Petersen knut_peter...@t-online.de
Cc: Dave Chinner da...@fromorbit.com
Cc: Glauber Costa glom...@openvz.org
Cc: Glauber Costa glom...@gmail.com
Cc: Andrew Morton a...@linux-foundation.org
Cc: Rik van Riel r...@redhat.com
Cc: Mel Gorman mgor...@suse.de
Cc: Johannes Weiner han...@cmpxchg.org
Cc: Michal Hocko mho...@suse.cz
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_gem.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index df9253d..cdfb9da 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4800,10 +4800,10 @@ i915_gem_inactive_count(struct shrinker *shrinker, 
struct shrink_control *sc)
 
if (!mutex_trylock(dev-struct_mutex)) {
if (!mutex_is_locked_by(dev-struct_mutex, current))
-   return SHRINK_STOP;
+   return 0;
 
if (dev_priv-mm.shrinker_no_lock_stealing)
-   return SHRINK_STOP;
+   return 0;
 
unlock = false;
}
@@ -4901,10 +4901,10 @@ i915_gem_inactive_scan(struct shrinker *shrinker, 
struct shrink_control *sc)
 
if (!mutex_trylock(dev-struct_mutex)) {
if (!mutex_is_locked_by(dev-struct_mutex, current))
-   return 0;
+   return SHRINK_STOP;
 
if (dev_priv-mm.shrinker_no_lock_stealing)
-   return 0;
+   return SHRINK_STOP;
 
unlock = false;
}
-- 
1.8.4.rc3

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


[Intel-gfx] [PATCH] drm/i915: we can't move_to_active before intel_ring_begin

2013-09-25 Thread Daniel Vetter
Otherwise we can die in a fire of not-yet-allocated lazy requests when
we expect them to be there:

[ 4405.463113] [ cut here ]
[ 4405.464769] kernel BUG at 
/home/user/src/linux/drivers/gpu/drm/i915/intel_ringbuffer.h:268!
[ 4405.466392] invalid opcode:  [#1] PREEMPT SMP
[ 4405.468007] Modules linked in: snd_hda_codec_hdmi hid_generic 
snd_hda_codec_realtek usbhid hid iTCO_wdt snd_hda_intel psmouse i2c_i801 
snd_hda_codec pcspkr snd_hwdep i915 snd_pcm snd_page_alloc snd_timer 
cfbfillrect snd cfbimgblt lpc_ich cfbcopyarea soundcore drm_kms_helper mfd_core 
evdev wmi
[ 4405.471387] CPU: 5 PID: 9300 Comm: gem_storedw_bat Not tainted 
3.12.0-rc2-drm_vbl+ #1
[ 4405.473047] Hardware name:  /DZ77BH-55K, BIOS 
BHZ7710H.86A.0100.2013.0517.0942 05/17/2013
[ 4405.474712] task: 8800618d4b00 ti: 88010a806000 task.ti: 
88010a806000
[ 4405.476370] RIP: 0010:[a009ffa9]  [a009ffa9] 
i915_vma_move_to_active+0x1b9/0x1e0 [i915]
[ 4405.478045] RSP: 0018:88010a807be8  EFLAGS: 00010246
[ 4405.479689] RAX: 88011a681000 RBX: 8800b364f9c0 RCX: 88011902b898
[ 4405.481321] RDX: 8800d4a1b6e0 RSI: 8800d4a1a8b8 RDI: 88011902b840
[ 4405.482932] RBP: 88010a807c08 R08: 0001 R09: 
[ 4405.484526] R10: 0001 R11:  R12: 8800d4a1a8b8
[ 4405.486100] R13:  R14: 8800d4a18000 R15: 8800b364f9c0
[ 4405.487664] FS:  7f36c1a738c0() GS:88011f34() 
knlGS:
[ 4405.489216] CS:  0010 DS:  ES:  CR0: 80050033
[ 4405.490747] CR2: 7fff1b28ea30 CR3: 000119e0d000 CR4: 001407e0
[ 4405.492276] Stack:
[ 4405.493774]  88010a807dd8 8800d4a1a8b8 8800d3c1c400 
a00ac060
[ 4405.495276]  88010a807d28 a00aa0db 88010a807cb8 
810aa4e4
[ 4405.496776]  0003 8801 8800618d50e8 
81a9da00
[ 4405.498265] Call Trace:
[ 4405.499735]  [a00ac060] ? gen6_ppgtt_insert_entries+0x1a0/0x1a0 
[i915]
[ 4405.501218]  [a00aa0db] 
i915_gem_do_execbuffer.clone.18+0x65b/0x12e0 [i915]
[ 4405.502685]  [810aa4e4] ? lock_release_non_nested+0xa4/0x360
[ 4405.504134]  [a00ab298] i915_gem_execbuffer2+0xa8/0x290 [i915]
[ 4405.505573]  [813552b9] drm_ioctl+0x419/0x5c0
[ 4405.506991]  [81128b12] ? handle_mm_fault+0x352/0xa00
[ 4405.508399]  [a00ab1f0] ? i915_gem_execbuffer+0x490/0x490 [i915]
[ 4405.509792]  [8103cd2c] ? __do_page_fault+0x1fc/0x4b0
[ 4405.511170]  [81165e66] do_vfs_ioctl+0x96/0x560
[ 4405.512533]  [81512163] ? error_sti+0x5/0x6
[ 4405.513878]  [81511d0d] ? retint_swapgs+0xe/0x13
[ 4405.515208]  [811663d1] SyS_ioctl+0xa1/0xb0
[ 4405.516522]  [8129ddee] ? trace_hardirqs_on_thunk+0x3a/0x3f
[ 4405.517830]  [81512792] system_call_fastpath+0x16/0x1b
[ 4405.519119] Code: f6 83 40 01 00 00 01 0f 85 11 ff ff ff b8 01 00 00 00 f0 
0f c1 03 ff c0 83 f8 01 7e 10 80 8b 40 01 00 00 01 e9 f5 fe ff ff 0f 0b 0f 0b 
80 3d 61 55 08 00 01 74 e7 be 2f 00 00 00 48 c7 c7 18 03
[ 4405.520610] RIP  [a009ffa9] i915_vma_move_to_active+0x1b9/0x1e0 
[i915]
[ 4405.522001]  RSP 88010a807be8
[ 4405.523367] [drm:intel_crtc_set_config], [CRTC:3] [FB:50] #connectors=1 (x 
y) (0 0)
[ 4405.524751] [drm:intel_set_config_compute_mode_changes], computed changes 
for [CRTC:3], mode_changed=0, fb_changed=0
[ 4405.526152] [drm:intel_modeset_stage_output_state], [CONNECTOR:16:HDMI-A-2] 
to [CRTC:3]
[ 4405.531096] [drm:intel_crtc_set_config], [CRTC:3] [FB:50] #connectors=1 (x 
y) (0 0)
[ 4405.532491] [drm:intel_set_config_compute_mode_changes], computed changes 
for [CRTC:3], mode_changed=0, fb_changed=0
[ 4405.533903] [drm:intel_modeset_stage_output_state], [CONNECTOR:16:HDMI-A-2] 
to [CRTC:3]
[ 4405.53] ---[ end trace 53d1b708421bb5b3 ]---

This regression has been introduced in from Ben Widawsky's ppgtt/vma
enabling patch drm/i915: Use the new vm [un]bind functions.

This should be exercised by
igt/gem_storedw_batches_loop/secure-dispatch.

v2: Clarify the copypasta comment and update it to suit the new
location of the move_to_active call for the batch vma.

Reported-by: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 34 --
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 88c924f..f540207 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -929,6 +929,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
drm_i915_private_t *dev_priv = 

[Intel-gfx] [PATCH] drm/i915: we can't call move_to_active before intel_ring_begin

2013-09-25 Thread Daniel Vetter
Otherwise we can die in a fire of not-yet-allocated lazy requests when
we expect them to be there:

[ 4405.463113] [ cut here ]
[ 4405.464769] kernel BUG at 
/home/user/src/linux/drivers/gpu/drm/i915/intel_ringbuffer.h:268!
[ 4405.466392] invalid opcode:  [#1] PREEMPT SMP
[ 4405.468007] Modules linked in: snd_hda_codec_hdmi hid_generic 
snd_hda_codec_realtek usbhid hid iTCO_wdt snd_hda_intel psmouse i2c_i801 
snd_hda_codec pcspkr snd_hwdep i915 snd_pcm snd_page_alloc snd_timer 
cfbfillrect snd cfbimgblt lpc_ich cfbcopyarea soundcore drm_kms_helper mfd_core 
evdev wmi
[ 4405.471387] CPU: 5 PID: 9300 Comm: gem_storedw_bat Not tainted 
3.12.0-rc2-drm_vbl+ #1
[ 4405.473047] Hardware name:  /DZ77BH-55K, BIOS 
BHZ7710H.86A.0100.2013.0517.0942 05/17/2013
[ 4405.474712] task: 8800618d4b00 ti: 88010a806000 task.ti: 
88010a806000
[ 4405.476370] RIP: 0010:[a009ffa9]  [a009ffa9] 
i915_vma_move_to_active+0x1b9/0x1e0 [i915]
[ 4405.478045] RSP: 0018:88010a807be8  EFLAGS: 00010246
[ 4405.479689] RAX: 88011a681000 RBX: 8800b364f9c0 RCX: 88011902b898
[ 4405.481321] RDX: 8800d4a1b6e0 RSI: 8800d4a1a8b8 RDI: 88011902b840
[ 4405.482932] RBP: 88010a807c08 R08: 0001 R09: 
[ 4405.484526] R10: 0001 R11:  R12: 8800d4a1a8b8
[ 4405.486100] R13:  R14: 8800d4a18000 R15: 8800b364f9c0
[ 4405.487664] FS:  7f36c1a738c0() GS:88011f34() 
knlGS:
[ 4405.489216] CS:  0010 DS:  ES:  CR0: 80050033
[ 4405.490747] CR2: 7fff1b28ea30 CR3: 000119e0d000 CR4: 001407e0
[ 4405.492276] Stack:
[ 4405.493774]  88010a807dd8 8800d4a1a8b8 8800d3c1c400 
a00ac060
[ 4405.495276]  88010a807d28 a00aa0db 88010a807cb8 
810aa4e4
[ 4405.496776]  0003 8801 8800618d50e8 
81a9da00
[ 4405.498265] Call Trace:
[ 4405.499735]  [a00ac060] ? gen6_ppgtt_insert_entries+0x1a0/0x1a0 
[i915]
[ 4405.501218]  [a00aa0db] 
i915_gem_do_execbuffer.clone.18+0x65b/0x12e0 [i915]
[ 4405.502685]  [810aa4e4] ? lock_release_non_nested+0xa4/0x360
[ 4405.504134]  [a00ab298] i915_gem_execbuffer2+0xa8/0x290 [i915]
[ 4405.505573]  [813552b9] drm_ioctl+0x419/0x5c0
[ 4405.506991]  [81128b12] ? handle_mm_fault+0x352/0xa00
[ 4405.508399]  [a00ab1f0] ? i915_gem_execbuffer+0x490/0x490 [i915]
[ 4405.509792]  [8103cd2c] ? __do_page_fault+0x1fc/0x4b0
[ 4405.511170]  [81165e66] do_vfs_ioctl+0x96/0x560
[ 4405.512533]  [81512163] ? error_sti+0x5/0x6
[ 4405.513878]  [81511d0d] ? retint_swapgs+0xe/0x13
[ 4405.515208]  [811663d1] SyS_ioctl+0xa1/0xb0
[ 4405.516522]  [8129ddee] ? trace_hardirqs_on_thunk+0x3a/0x3f
[ 4405.517830]  [81512792] system_call_fastpath+0x16/0x1b
[ 4405.519119] Code: f6 83 40 01 00 00 01 0f 85 11 ff ff ff b8 01 00 00 00 f0 
0f c1 03 ff c0 83 f8 01 7e 10 80 8b 40 01 00 00 01 e9 f5 fe ff ff 0f 0b 0f 0b 
80 3d 61 55 08 00 01 74 e7 be 2f 00 00 00 48 c7 c7 18 03
[ 4405.520610] RIP  [a009ffa9] i915_vma_move_to_active+0x1b9/0x1e0 
[i915]
[ 4405.522001]  RSP 88010a807be8
[ 4405.523367] [drm:intel_crtc_set_config], [CRTC:3] [FB:50] #connectors=1 (x 
y) (0 0)
[ 4405.524751] [drm:intel_set_config_compute_mode_changes], computed changes 
for [CRTC:3], mode_changed=0, fb_changed=0
[ 4405.526152] [drm:intel_modeset_stage_output_state], [CONNECTOR:16:HDMI-A-2] 
to [CRTC:3]
[ 4405.531096] [drm:intel_crtc_set_config], [CRTC:3] [FB:50] #connectors=1 (x 
y) (0 0)
[ 4405.532491] [drm:intel_set_config_compute_mode_changes], computed changes 
for [CRTC:3], mode_changed=0, fb_changed=0
[ 4405.533903] [drm:intel_modeset_stage_output_state], [CONNECTOR:16:HDMI-A-2] 
to [CRTC:3]
[ 4405.53] ---[ end trace 53d1b708421bb5b3 ]---

This regression has been introduced in from Ben Widawsky's ppgtt/vma
enabling patch drm/i915: Use the new vm [un]bind functions.

This should be exercised by
igt/gem_storedw_batches_loop/secure-dispatch.

Note that this won't fix the full ordeal for real ppgtt since the
potential allocation for the batch vma could recurse into the shrinker
and wreak utter havoc with our carefully reserved buffers. But for now
this is good enough.

The real fix involves some trickery to allocate the batch vma around
the call to eb_lookup_vmas and do all the additional buffer space
reserving for the batch in global gtt in the normal reservation step.

But that requires quite some frobbing of various pieces of code, so
definitely a 2nd step.

v2: Clarify the copypasta comment and update it to suit the new
location of the move_to_active call for the batch vma.

v3: Pimp the commit message a bit to explain that this isn't the full
fix.

Reported-by: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Chris Wilson 

Re: [Intel-gfx] [PATCH v3 0/4] Fix Win8 backlight issue

2013-09-25 Thread Jörg Otte
Backlight can't be modified with this patch set -  neither with function
keys nor
with the gui. It is a step backward to v3.11-rc1

Video driver: i915
FUJITSU LIFEBOOK AH532/FJNBB1C, BIOS Version 1.09 05/22/2012

acpi_backlight=video works.

Jörg


2013/9/24 Igor Gnatenko i.gnatenko.br...@gmail.com

 On Tue, 2013-09-24 at 17:47 +0800, Aaron Lu wrote:
  v3:
  1 Add a new patch 4/4 to fix some problems in thinkpad-acpi module;
  2 Remove unnecessary function acpi_video_unregister introduced in
patch 2/3 as pointed out by Jani Nikula.
 
  v2:
  v1 has the subject of Rework ACPI video driver and is posted here:
  http://lkml.org/lkml/2013/9/9/74
  Since the objective is really to fix Win8 backlight issues, I changed
  the subject in this version, sorry about that.
 
  This patchset has three patches, the first introduced a new API named
  backlight_device_registered in backlight layer that can be used for
  backlight interface provider module to check if a specific type backlight
  interface has been registered, see changelog for patch 1/3 for details.
  Then patch 2/3 does the cleanup to sepeate the backlight control and
  event delivery functionality in the ACPI video module and patch 3/3
  solves some Win8 backlight control problems by avoiding register ACPI
  video's backlight interface if:
  1 Kernel cmdline option acpi_backlight=video is not given;
  2 This is a Win8 system;
  3 Native backlight control interface exists.
 
  Technically, patch 2/3 is not required to fix the issue here. So if you
  think it is not necessary, I can remove it from the series.
 
  Aaron Lu (4):
backlight: introduce backlight_device_registered
ACPI / video: seperate backlight control and event interface
ACPI / video: Do not register backlight if win8 and native interface
  exists
thinkpad-acpi: fix handle locate for video and query of _BCL
 
   drivers/acpi/internal.h  |   5 +-
   drivers/acpi/video.c | 442
 ---
   drivers/acpi/video_detect.c  |  14 +-
   drivers/platform/x86/thinkpad_acpi.c |  31 ++-
   drivers/video/backlight/backlight.c  |  31 +++
   include/acpi/video.h |   2 +
   include/linux/backlight.h|   4 +
   7 files changed, 324 insertions(+), 205 deletions(-)
 

 Excellent! I've tested this patch-set on ThinkPad X230 and backlight
 issue is exhausted.

 Thanks.

 --
 Igor Gnatenko
 Fedora release 20 (Heisenbug)
 Linux 3.11.1-300.fc20.x86_64


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


Re: [Intel-gfx] BUG: sleeping function called from invalid context on 3.10.10-rt7

2013-09-25 Thread Steven Rostedt
On Wed, 25 Sep 2013 06:32:10 +0200
Mario Kleiner mario.klei...@tuebingen.mpg.de wrote:


 I assume if a spin_lock_irqsave doesn't really disable interrupts on a 
 RT kernel with normal spinlocks then local_irq_disable won't really 
 disable interrupts either?
 

That is incorrect. On PREEMPT_RT, you are right about
spin_lock_irqsave() not disabling interrupts, but local_irq_disable()
does indeed disable interrupts.

Open coded local_irq_disable() usually ends up being a bug as it does
nothing to synchronize interrupts from other CPUs. But most of those
bugs have been removed, and there are some very legit reasons for using
local_irq_disable(). PREEMPT_RT honors those.

The reason PREEMPT_RT does not disable interrupts for
spin_lock_irqsave(), is because that's usually used for when a
interrupt uses the same spinlock. You need the irqsave() part in order
to prevent a deadlock, if the code that has the spinlock gets preempted
by the interrupt and that interrupt tries to grab the same lock.

Because PREEMPT_RT runs interrupts as threads, we don't have that issue,
because if the interrupt preempts the holder of the lock and it tries
to grab the same lock, it will just block like any other thread trying
to grab that lock. That is, spinlocks turn into mutexes on PREEMPT_RT.

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


Re: [Intel-gfx] BUG: sleeping function called from invalid context on 3.10.10-rt7

2013-09-25 Thread Steven Rostedt
On Wed, 25 Sep 2013 06:32:10 +0200
Mario Kleiner mario.klei...@tuebingen.mpg.de wrote:

 
 But given the new situation, your proposal is great! If we push the 
 clock readouts into the get_scanoutpos routine, we can make this robust 
 without causing grief for the rt people and without the need for a new 
 separate lock for display regs in intel-kms.
 
 E.g., for intel-kms:
 
 i915_get_crtc_scanoutpos(..., ktime_t *stime, ktime_t *etime)
 {
...
spin_lock_irqsave(...uncore.lock);
preempt_disable();
*stime = ktime_get();
position = __raw_i915_read32(dev_priv, PIPEDSL(pipe));
*etime = ktime_get();
preempt_enable();
spin_unlock_irqrestore(...uncore.lock)
...
 }
 
 With your patchset to reduce the amount of register reads needed in that 
 function, and given that forcewake handling isn't needed for these 
 registers, this should make it robust again and wouldn't need new locks.
 
 Unless ktime_get is also a bad thing to do in a preempt disabled section?

ktime_get() works fine in preempt_disable sections, although it may add
some latencies, but you shouldn't need to worry about it.

I like this solution the best too, but if it does go in, I would ask to
send us the patch for adding the preempt_disable() and we can add the
preempt_disable_rt() to it. Why make mainline have a little more
overhead?

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


Re: [Intel-gfx] BUG: sleeping function called from invalid context on 3.10.10-rt7

2013-09-25 Thread Steven Rostedt

Sorry for the late reply, I was at Linux Plumbers, and had a bunch of
stuff to catch up on when I returned.

On Sat, 21 Sep 2013 00:07:36 +0200
Mario Kleiner mario.klei...@tuebingen.mpg.de wrote:

 
 Steven, would it then be acceptable to convert that faster lock into a 
 raw_spinlock_t or is this unacceptable? If so, the preempt_disable() 
 could stay, right?

If a spinlock is tight (not held for more than 2us on todays
processors), and has little contention, than I would be fine with
converting it to raw. And if that's the only lock held you could do the
preempt_disable() call.

In fact, if you want, you can leave the preempt_disable() out of
mainline, and send a patch to us that uses preempt_disable_rt() and
add a comment to it. In the -rt patch, preempt_disable_rt() is a nop
when PREEMPT_RT is not set, and is preempt_disable() when it is.


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


Re: [Intel-gfx] BUG: sleeping function called from invalid context on 3.10.10-rt7

2013-09-25 Thread Steven Rostedt
On Wed, 25 Sep 2013 10:49:36 +0300
Ville Syrjälä ville.syrj...@linux.intel.com wrote:

 
 The preempt_disable/enable is not needed. The spinlock serves the same
 purpose already.

As stated, that was only for the -rt patch, as spin_lock_irqsave does
not disable preemption nor does it even disable interrupts.

But I agree, the added preempt_disable() should be sent to us to keep
in the -rt patch itself. We really appreciate that you are thinking
about us :-)  But something like this will just confuse the mainline
folks. Having a preempt_disable_rt() would make a lot more sense
(which exists in the -rt patch).

 
 As far as ktime_get(), I've used it from spinlocked/irq disabled sections
 and so far haven't seen it do bad things. But would be nice to get some
 official statement to that effect.

It's just a read seqlock. It may do a few loops to get the correct
time, but it's fine to have in a preempt/irq disabled section.

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


[Intel-gfx] [PATCH 01/22] drm: Move the GET_CAP macros next to the corresponding ioctl structure

2013-09-25 Thread Damien Lespiau
It's a tiny bit more logical to find the different capabilities you can
use with the GET_CAP ioctl next to the structure rather than putting
them at the end of the file.

v2: Tab align the litterals (David Herrmann)
v3: Make it clearer that DRM_PRIME_CAP_EXPORT/IMPORT are flags of
DRM_CAP_PRIME.
v4: Rebase on top of latest bits (DRM_CAP_ASYNC_PAGE_FLIP was
introduced)

Reviewed-by: David Herrmann dh.herrm...@gmail.com (for v2)
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 include/uapi/drm/drm.h | 21 ++---
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index ece8678..1e09e8f 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -611,6 +611,16 @@ struct drm_gem_open {
__u64 size;
 };
 
+#define DRM_CAP_DUMB_BUFFER0x1
+#define DRM_CAP_VBLANK_HIGH_CRTC   0x2
+#define DRM_CAP_DUMB_PREFERRED_DEPTH   0x3
+#define DRM_CAP_DUMB_PREFER_SHADOW 0x4
+#define DRM_CAP_PRIME  0x5
+#define  DRM_PRIME_CAP_IMPORT  0x1
+#define  DRM_PRIME_CAP_EXPORT  0x2
+#define DRM_CAP_TIMESTAMP_MONOTONIC0x6
+#define DRM_CAP_ASYNC_PAGE_FLIP0x7
+
 /** DRM_IOCTL_GET_CAP ioctl argument type */
 struct drm_get_cap {
__u64 capability;
@@ -774,17 +784,6 @@ struct drm_event_vblank {
__u32 reserved;
 };
 
-#define DRM_CAP_DUMB_BUFFER 0x1
-#define DRM_CAP_VBLANK_HIGH_CRTC 0x2
-#define DRM_CAP_DUMB_PREFERRED_DEPTH 0x3
-#define DRM_CAP_DUMB_PREFER_SHADOW 0x4
-#define DRM_CAP_PRIME 0x5
-#define DRM_CAP_TIMESTAMP_MONOTONIC 0x6
-#define DRM_CAP_ASYNC_PAGE_FLIP 0x7
-
-#define DRM_PRIME_CAP_IMPORT 0x1
-#define DRM_PRIME_CAP_EXPORT 0x2
-
 /* typedef area */
 #ifndef __KERNEL__
 typedef struct drm_clip_rect drm_clip_rect_t;
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 02/22] drm: Add a SET_CLIENT_CAP ioctl

2013-09-25 Thread Damien Lespiau
This ioctl can be used to turn some knobs in a DRM driver. The client
can ask the DRM core for an alternate view of the reality: it can be
useful to be able to instruct the core that the DRM client can handle
new functionnality that would otherwise break current ABI.

v2: Rename to ioctl from SET_CAP to SET_CLIENT_CAP (Chris Wilson)

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/drm_drv.c   | 1 +
 drivers/gpu/drm/drm_ioctl.c | 9 +
 include/drm/drmP.h  | 2 ++
 include/uapi/drm/drm.h  | 7 +++
 4 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index e572dd2..e79d8d9 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -69,6 +69,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_GET_CLIENT, drm_getclient, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_GET_STATS, drm_getstats, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_GET_CAP, drm_getcap, 
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF(DRM_IOCTL_SET_CLIENT_CAP, drm_setclientcap, 0),
DRM_IOCTL_DEF(DRM_IOCTL_SET_VERSION, drm_setversion, DRM_MASTER),
 
DRM_IOCTL_DEF(DRM_IOCTL_SET_UNIQUE, drm_setunique, 
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 07247e2..15da412 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -303,6 +303,15 @@ int drm_getcap(struct drm_device *dev, void *data, struct 
drm_file *file_priv)
 }
 
 /**
+ * Set device/driver capabilities
+ */
+int
+drm_setclientcap(struct drm_device *dev, void *data, struct drm_file 
*file_priv)
+{
+   return -EINVAL;
+}
+
+/**
  * Setversion ioctl.
  *
  * \param inode device inode.
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index b46fb45..dbc86b0 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1303,6 +1303,8 @@ extern int drm_getstats(struct drm_device *dev, void 
*data,
struct drm_file *file_priv);
 extern int drm_getcap(struct drm_device *dev, void *data,
  struct drm_file *file_priv);
+extern int drm_setclientcap(struct drm_device *dev, void *data,
+   struct drm_file *file_priv);
 extern int drm_setversion(struct drm_device *dev, void *data,
  struct drm_file *file_priv);
 extern int drm_noop(struct drm_device *dev, void *data,
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 1e09e8f..526baed 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -627,6 +627,12 @@ struct drm_get_cap {
__u64 value;
 };
 
+/** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
+struct drm_set_client_cap {
+   __u64 capability;
+   __u64 value;
+};
+
 #define DRM_CLOEXEC O_CLOEXEC
 struct drm_prime_handle {
__u32 handle;
@@ -659,6 +665,7 @@ struct drm_prime_handle {
 #define DRM_IOCTL_GEM_FLINKDRM_IOWR(0x0a, struct drm_gem_flink)
 #define DRM_IOCTL_GEM_OPEN DRM_IOWR(0x0b, struct drm_gem_open)
 #define DRM_IOCTL_GET_CAP  DRM_IOWR(0x0c, struct drm_get_cap)
+#define DRM_IOCTL_SET_CLIENT_CAP   DRM_IOW( 0x0d, struct 
drm_set_client_cap)
 
 #define DRM_IOCTL_SET_UNIQUE   DRM_IOW( 0x10, struct drm_unique)
 #define DRM_IOCTL_AUTH_MAGIC   DRM_IOW( 0x11, struct drm_auth)
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 03/22] drm: Add HDMI stereo 3D flags to struct drm_mode_modeinfo

2013-09-25 Thread Damien Lespiau
HDMI 1.4a defines a few layouts that we'd like to expose. This commits
add new modeinfo flags that can be used to list the supported stereo
layouts (when querying the list of modes) and to set a given stereo 3D
mode (when setting a mode).

v2: Add a drm_mode_is_stereo() helper

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 include/drm/drm_crtc.h  | 14 ++
 include/uapi/drm/drm_mode.h | 36 ++--
 2 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 24f4995..825d6fa 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -180,6 +180,20 @@ struct drm_display_mode {
int hsync;  /* in kHz */
 };
 
+#define DRM_MODE_FLAG_3D_MASK  (DRM_MODE_FLAG_3D_FRAME_PACKING | \
+DRM_MODE_FLAG_3D_FIELD_ALTERNATIVE | \
+DRM_MODE_FLAG_3D_LINE_ALTERNATIVE  | \
+DRM_MODE_FLAG_3D_SIDE_BY_SIDE_FULL | \
+DRM_MODE_FLAG_3D_L_DEPTH   | \
+DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH | \
+DRM_MODE_FLAG_3D_TOP_AND_BOTTOM| \
+DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF)
+
+static inline bool drm_mode_is_stereo(const struct drm_display_mode *mode)
+{
+   return mode-flags  DRM_MODE_FLAG_3D_MASK;
+}
+
 enum drm_connector_status {
connector_status_connected = 1,
connector_status_disconnected = 2,
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 113d324..bafe612 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -44,20 +44,28 @@
 
 /* Video mode flags */
 /* bit compatible with the xorg definitions. */
-#define DRM_MODE_FLAG_PHSYNC   (10)
-#define DRM_MODE_FLAG_NHSYNC   (11)
-#define DRM_MODE_FLAG_PVSYNC   (12)
-#define DRM_MODE_FLAG_NVSYNC   (13)
-#define DRM_MODE_FLAG_INTERLACE(14)
-#define DRM_MODE_FLAG_DBLSCAN  (15)
-#define DRM_MODE_FLAG_CSYNC(16)
-#define DRM_MODE_FLAG_PCSYNC   (17)
-#define DRM_MODE_FLAG_NCSYNC   (18)
-#define DRM_MODE_FLAG_HSKEW(19) /* hskew provided */
-#define DRM_MODE_FLAG_BCAST(110)
-#define DRM_MODE_FLAG_PIXMUX   (111)
-#define DRM_MODE_FLAG_DBLCLK   (112)
-#define DRM_MODE_FLAG_CLKDIV2  (113)
+#define DRM_MODE_FLAG_PHSYNC   (10)
+#define DRM_MODE_FLAG_NHSYNC   (11)
+#define DRM_MODE_FLAG_PVSYNC   (12)
+#define DRM_MODE_FLAG_NVSYNC   (13)
+#define DRM_MODE_FLAG_INTERLACE(14)
+#define DRM_MODE_FLAG_DBLSCAN  (15)
+#define DRM_MODE_FLAG_CSYNC(16)
+#define DRM_MODE_FLAG_PCSYNC   (17)
+#define DRM_MODE_FLAG_NCSYNC   (18)
+#define DRM_MODE_FLAG_HSKEW(19) /* hskew provided */
+#define DRM_MODE_FLAG_BCAST(110)
+#define DRM_MODE_FLAG_PIXMUX   (111)
+#define DRM_MODE_FLAG_DBLCLK   (112)
+#define DRM_MODE_FLAG_CLKDIV2  (113)
+#define DRM_MODE_FLAG_3D_FRAME_PACKING (114)
+#define DRM_MODE_FLAG_3D_FIELD_ALTERNATIVE (115)
+#define DRM_MODE_FLAG_3D_LINE_ALTERNATIVE  (116)
+#define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_FULL (117)
+#define DRM_MODE_FLAG_3D_L_DEPTH   (118)
+#define DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH (119)
+#define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM(120)
+#define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF (121)
 
 /* DPMS flags */
 /* bit compatible with the xorg definitions. */
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 04/22] drm: Add a STEREO_3D capability to the SET_CLIENT_CAP ioctl

2013-09-25 Thread Damien Lespiau
This capability allows user space to control the delivery of modes with
the 3D flags set. This is to not play games with current user space
users not knowing anything about stereo 3D flags and that could try
to set a mode with one or several of those bits set.

So, the plan is to remove the stereo modes from the list of modes we
give to DRM clients by default, and let them through if we are being
told otherwise.

stereo_allowed is bound to the drm_file structure to make it a
per-client setting, not a global one.

v2: Replace clearing 3D flags by discarding the stereo modes now that
they are regular modes.
v3: SET_CAP - SET_CLIENT_CAP rename (Chris Wilson)

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/drm_crtc.c  | 19 ++-
 drivers/gpu/drm/drm_ioctl.c | 14 +-
 include/drm/drmP.h  |  3 +++
 include/uapi/drm/drm.h  |  9 +
 4 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e79577c..454ac8a 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1581,6 +1581,19 @@ out:
return ret;
 }
 
+static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
+const struct drm_file *file_priv)
+{
+   /*
+* If user-space hasn't configured the driver to expose the stereo 3D
+* modes, don't expose them.
+*/
+   if (!file_priv-stereo_allowed  drm_mode_is_stereo(mode))
+   return false;
+
+   return true;
+}
+
 /**
  * drm_mode_getconnector - get connector configuration
  * @dev: drm device for the ioctl
@@ -1646,7 +1659,8 @@ int drm_mode_getconnector(struct drm_device *dev, void 
*data,
 
/* delayed so we get modes regardless of pre-fill_modes state */
list_for_each_entry(mode, connector-modes, head)
-   mode_count++;
+   if (drm_mode_expose_to_userspace(mode, file_priv))
+   mode_count++;
 
out_resp-connector_id = connector-base.id;
out_resp-connector_type = connector-connector_type;
@@ -1668,6 +1682,9 @@ int drm_mode_getconnector(struct drm_device *dev, void 
*data,
copied = 0;
mode_ptr = (struct drm_mode_modeinfo __user *)(unsigned 
long)out_resp-modes_ptr;
list_for_each_entry(mode, connector-modes, head) {
+   if (!drm_mode_expose_to_userspace(mode, file_priv))
+   continue;
+
drm_crtc_convert_to_umode(u_mode, mode);
if (copy_to_user(mode_ptr + copied,
 u_mode, sizeof(u_mode))) {
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 15da412..dffc836 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -308,7 +308,19 @@ int drm_getcap(struct drm_device *dev, void *data, struct 
drm_file *file_priv)
 int
 drm_setclientcap(struct drm_device *dev, void *data, struct drm_file 
*file_priv)
 {
-   return -EINVAL;
+   struct drm_set_client_cap *req = data;
+
+   switch (req-capability) {
+   case DRM_CLIENT_CAP_STEREO_3D:
+   if (req-value  1)
+   return -EINVAL;
+   file_priv-stereo_allowed = req-value;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   return 0;
 }
 
 /**
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index dbc86b0..c65f496 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -433,6 +433,9 @@ struct drm_file {
struct drm_master *master; /* master this node is currently associated 
with
  N.B. not always minor-master */
 
+   /* true when the client has asked us to expose stereo 3D mode flags */
+   bool stereo_allowed;
+
/**
 * fbs - List of framebuffers associated with this file.
 *
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 526baed..9b24d65 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -627,6 +627,15 @@ struct drm_get_cap {
__u64 value;
 };
 
+/**
+ * DRM_CLIENT_CAP_STEREO_3D
+ *
+ * if set to 1, the DRM core will expose the stereo 3D capabilities of the
+ * monitor by advertising the supported 3D layouts in the flags of struct
+ * drm_mode_modeinfo.
+ */
+#define DRM_CLIENT_CAP_STEREO_3D   1
+
 /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
 struct drm_set_client_cap {
__u64 capability;
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 05/22] drm/edid: Expose mandatory stereo modes for HDMI sinks

2013-09-25 Thread Damien Lespiau
For now, let's just look at the 3D_present flag of the CEA HDMI vendor
block to detect if the sink supports a small list of then mandatory 3D
formats.

See the HDMI 1.4a 3D extraction for detail:
  http://www.hdmi.org/manufacturer/specification.aspx

v2: Rename freq to vrefresh, make the mandatory structure a bit more
compact, fix some white space issues and add a couple of const
(Ville Syrjälä)

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/drm_edid.c | 110 ++---
 1 file changed, 103 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 1688ff5..52e6087 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2553,13 +2553,95 @@ do_cea_modes(struct drm_connector *connector, const u8 
*db, u8 len)
return modes;
 }
 
+struct stereo_mandatory_mode {
+   int width, height, vrefresh;
+   unsigned int flags;
+};
+
+static const struct stereo_mandatory_mode stereo_mandatory_modes[] = {
+   { 1920, 1080, 24,
+ DRM_MODE_FLAG_3D_TOP_AND_BOTTOM | DRM_MODE_FLAG_3D_FRAME_PACKING },
+   { 1920, 1080, 50,
+ DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF },
+   { 1920, 1080, 60,
+ DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF },
+   { 1280, 720,  50,
+ DRM_MODE_FLAG_3D_TOP_AND_BOTTOM | DRM_MODE_FLAG_3D_FRAME_PACKING },
+   { 1280, 720,  60,
+ DRM_MODE_FLAG_3D_TOP_AND_BOTTOM | DRM_MODE_FLAG_3D_FRAME_PACKING }
+};
+
+static bool
+stereo_match_mandatory(const struct drm_display_mode *mode,
+  const struct stereo_mandatory_mode *stereo_mode)
+{
+   unsigned int interlaced = mode-flags  DRM_MODE_FLAG_INTERLACE;
+
+   return mode-hdisplay == stereo_mode-width 
+  mode-vdisplay == stereo_mode-height 
+  interlaced == (stereo_mode-flags  DRM_MODE_FLAG_INTERLACE) 
+  drm_mode_vrefresh(mode) == stereo_mode-vrefresh;
+}
+
+static const struct stereo_mandatory_mode *
+hdmi_find_stereo_mandatory_mode(const struct drm_display_mode *mode)
+{
+   int i;
+
+   for (i = 0; i  ARRAY_SIZE(stereo_mandatory_modes); i++)
+   if (stereo_match_mandatory(mode, stereo_mandatory_modes[i]))
+   return stereo_mandatory_modes[i];
+
+   return NULL;
+}
+
+static int add_hdmi_mandatory_stereo_modes(struct drm_connector *connector)
+{
+   struct drm_device *dev = connector-dev;
+   const struct drm_display_mode *mode;
+   struct list_head stereo_modes;
+   int modes = 0;
+
+   INIT_LIST_HEAD(stereo_modes);
+
+   list_for_each_entry(mode, connector-probed_modes, head) {
+   const struct stereo_mandatory_mode *mandatory;
+   u32 stereo_layouts, layout;
+
+   mandatory = hdmi_find_stereo_mandatory_mode(mode);
+   if (!mandatory)
+   continue;
+
+   stereo_layouts = mandatory-flags  DRM_MODE_FLAG_3D_MASK;
+   do {
+   struct drm_display_mode *new_mode;
+
+   layout = 1  (ffs(stereo_layouts) - 1);
+   stereo_layouts = ~layout;
+
+   new_mode = drm_mode_duplicate(dev, mode);
+   if (!new_mode)
+   continue;
+
+   new_mode-flags |= layout;
+   list_add_tail(new_mode-head, stereo_modes);
+   modes++;
+   } while (stereo_layouts);
+   }
+
+   list_splice_tail(stereo_modes, connector-probed_modes);
+
+   return modes;
+}
+
 /*
  * do_hdmi_vsdb_modes - Parse the HDMI Vendor Specific data block
  * @connector: connector corresponding to the HDMI sink
  * @db: start of the CEA vendor specific block
  * @len: length of the CEA block payload, ie. one can access up to db[len]
  *
- * Parses the HDMI VSDB looking for modes to add to @connector.
+ * Parses the HDMI VSDB looking for modes to add to @connector. This function
+ * also adds the stereo 3d modes when applicable.
  */
 static int
 do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len)
@@ -2585,10 +2667,15 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, 
const u8 *db, u8 len)
 
/* the declared length is not long enough for the 2 first bytes
 * of additional video format capabilities */
-   offset += 2;
-   if (len  (8 + offset))
+   if (len  (8 + offset + 2))
goto out;
 
+   /* 3D_Present */
+   offset++;
+   if (db[8 + offset]  (1  7))
+   modes += add_hdmi_mandatory_stereo_modes(connector);
+
+   offset++;
vic_len = db[8 + offset]  5;
 
for (i = 0; i  vic_len  len = (9 + offset + i); i++) {
@@ -2668,8 +2755,8 @@ static int
 add_cea_modes(struct drm_connector 

[Intel-gfx] [PATCH 06/22] drm: Extract add_hdmi_mode() out of do_hdmi_vsdb_modes()

2013-09-25 Thread Damien Lespiau
So we respect a nice design of having similar functions at the same
level, in this case:

do_hdmi_vsdb_modes()
  - add_hdmi_mandatory_stereo_modes()
  - add_hdmi_mode()

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/drm_edid.c | 36 +---
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 52e6087..7366007 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2634,6 +2634,26 @@ static int add_hdmi_mandatory_stereo_modes(struct 
drm_connector *connector)
return modes;
 }
 
+static int add_hdmi_mode(struct drm_connector *connector, u8 vic)
+{
+   struct drm_device *dev = connector-dev;
+   struct drm_display_mode *newmode;
+
+   vic--; /* VICs start at 1 */
+   if (vic = ARRAY_SIZE(edid_4k_modes)) {
+   DRM_ERROR(Unknown HDMI VIC: %d\n, vic);
+   return 0;
+   }
+
+   newmode = drm_mode_duplicate(dev, edid_4k_modes[vic]);
+   if (!newmode)
+   return 0;
+
+   drm_mode_probed_add(connector, newmode);
+
+   return 1;
+}
+
 /*
  * do_hdmi_vsdb_modes - Parse the HDMI Vendor Specific data block
  * @connector: connector corresponding to the HDMI sink
@@ -2646,7 +2666,6 @@ static int add_hdmi_mandatory_stereo_modes(struct 
drm_connector *connector)
 static int
 do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len)
 {
-   struct drm_device *dev = connector-dev;
int modes = 0, offset = 0, i;
u8 vic_len;
 
@@ -2679,23 +2698,10 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, 
const u8 *db, u8 len)
vic_len = db[8 + offset]  5;
 
for (i = 0; i  vic_len  len = (9 + offset + i); i++) {
-   struct drm_display_mode *newmode;
u8 vic;
 
vic = db[9 + offset + i];
-
-   vic--; /* VICs start at 1 */
-   if (vic = ARRAY_SIZE(edid_4k_modes)) {
-   DRM_ERROR(Unknown HDMI VIC: %d\n, vic);
-   continue;
-   }
-
-   newmode = drm_mode_duplicate(dev, edid_4k_modes[vic]);
-   if (!newmode)
-   continue;
-
-   drm_mode_probed_add(connector, newmode);
-   modes++;
+   modes += add_hdmi_mode(connector, vic);
}
 
 out:
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 07/22] drm: Reject modes with more than 1 stereo flags set

2013-09-25 Thread Damien Lespiau
When setting a stereo 3D mode, there can be only one bit set describing
the layout of the frambuffer(s). So reject invalid modes early.

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/drm_crtc.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 454ac8a..090415f 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1319,6 +1319,10 @@ static int drm_crtc_convert_umode(struct 
drm_display_mode *out,
if (in-clock  INT_MAX || in-vrefresh  INT_MAX)
return -ERANGE;
 
+   /* At most, 1 set bit describing the 3D layout of the mode */
+   if (hweight32(in-flags  DRM_MODE_FLAG_3D_MASK)  1)
+   return -EINVAL;
+
out-clock = in-clock;
out-hdisplay = in-hdisplay;
out-hsync_start = in-hsync_start;
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 09/22] drm: Make drm_match_cea_mode() return the underlying 2D VIC for 3d modes

2013-09-25 Thread Damien Lespiau
When scanning out a stereo mode, the AVI infoframe vic field has to be
the underlyng 2D VIC. Before that commit, we weren't matching the CEA
mode because of the extra stereo flag and then were setting the VIC
field in the AVI infoframe to 0.

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/drm_edid.c  |  4 ++--
 drivers/gpu/drm/drm_modes.c | 18 --
 include/drm/drm_crtc.h  |  2 +-
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0bae76d..48f1746 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2404,7 +2404,7 @@ u8 drm_match_cea_mode(const struct drm_display_mode 
*to_match)
 
if ((KHZ2PICOS(to_match-clock) == KHZ2PICOS(clock1) ||
 KHZ2PICOS(to_match-clock) == KHZ2PICOS(clock2)) 
-   drm_mode_equal_no_clocks(to_match, cea_mode))
+   drm_mode_equal_no_clocks_no_stereo(to_match, cea_mode))
return mode + 1;
}
return 0;
@@ -2453,7 +2453,7 @@ static u8 drm_match_hdmi_mode(const struct 
drm_display_mode *to_match)
 
if ((KHZ2PICOS(to_match-clock) == KHZ2PICOS(clock1) ||
 KHZ2PICOS(to_match-clock) == KHZ2PICOS(clock2)) 
-   drm_mode_equal_no_clocks(to_match, hdmi_mode))
+   drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode))
return mode + 1;
}
return 0;
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index fc2adb6..c2cb2c8 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -830,12 +830,16 @@ bool drm_mode_equal(const struct drm_display_mode *mode1, 
const struct drm_displ
} else if (mode1-clock != mode2-clock)
return false;
 
-   return drm_mode_equal_no_clocks(mode1, mode2);
+   if ((mode1-flags  DRM_MODE_FLAG_3D_MASK) !=
+   (mode2-flags  DRM_MODE_FLAG_3D_MASK))
+   return false;
+
+   return drm_mode_equal_no_clocks_no_stereo(mode1, mode2);
 }
 EXPORT_SYMBOL(drm_mode_equal);
 
 /**
- * drm_mode_equal_no_clocks - test modes for equality
+ * drm_mode_equal_no_clocks_no_stereo - test modes for equality
  * @mode1: first mode
  * @mode2: second mode
  *
@@ -843,12 +847,13 @@ EXPORT_SYMBOL(drm_mode_equal);
  * None.
  *
  * Check to see if @mode1 and @mode2 are equivalent, but
- * don't check the pixel clocks.
+ * don't check the pixel clocks nor the stereo layout.
  *
  * RETURNS:
  * True if the modes are equal, false otherwise.
  */
-bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const 
struct drm_display_mode *mode2)
+bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1,
+   const struct drm_display_mode *mode2)
 {
if (mode1-hdisplay == mode2-hdisplay 
mode1-hsync_start == mode2-hsync_start 
@@ -860,12 +865,13 @@ bool drm_mode_equal_no_clocks(const struct 
drm_display_mode *mode1, const struct
mode1-vsync_end == mode2-vsync_end 
mode1-vtotal == mode2-vtotal 
mode1-vscan == mode2-vscan 
-   mode1-flags == mode2-flags)
+   (mode1-flags  ~DRM_MODE_FLAG_3D_MASK) ==
+(mode2-flags  ~DRM_MODE_FLAG_3D_MASK))
return true;
 
return false;
 }
-EXPORT_SYMBOL(drm_mode_equal_no_clocks);
+EXPORT_SYMBOL(drm_mode_equal_no_clocks_no_stereo);
 
 /**
  * drm_mode_validate_size - make sure modes adhere to size constraints
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 825d6fa..6b7f9c7 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -989,7 +989,7 @@ extern void drm_mode_config_reset(struct drm_device *dev);
 extern void drm_mode_config_cleanup(struct drm_device *dev);
 extern void drm_mode_set_name(struct drm_display_mode *mode);
 extern bool drm_mode_equal(const struct drm_display_mode *mode1, const struct 
drm_display_mode *mode2);
-extern bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, 
const struct drm_display_mode *mode2);
+extern bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode 
*mode1, const struct drm_display_mode *mode2);
 extern int drm_mode_width(const struct drm_display_mode *mode);
 extern int drm_mode_height(const struct drm_display_mode *mode);
 
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 08/22] drm: Set the relevant infoframe field when scanning out a 3D mode

2013-09-25 Thread Damien Lespiau
When scanning out a 3D mode on HDMI, we need to send an HDMI infoframe
with the corresponding layout to the sink.

v2: Make s3d_structure_from_display_mode() less subtle (Ville Syrjälä)

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/drm_edid.c | 40 ++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 7366007..0bae76d 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3421,6 +3421,33 @@ drm_hdmi_avi_infoframe_from_display_mode(struct 
hdmi_avi_infoframe *frame,
 }
 EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode);
 
+static enum hdmi_3d_structure
+s3d_structure_from_display_mode(const struct drm_display_mode *mode)
+{
+   u32 layout = mode-flags  DRM_MODE_FLAG_3D_MASK;
+
+   switch (layout) {
+   case DRM_MODE_FLAG_3D_FRAME_PACKING:
+   return HDMI_3D_STRUCTURE_FRAME_PACKING;
+   case DRM_MODE_FLAG_3D_FIELD_ALTERNATIVE:
+   return HDMI_3D_STRUCTURE_FIELD_ALTERNATIVE;
+   case DRM_MODE_FLAG_3D_LINE_ALTERNATIVE:
+   return HDMI_3D_STRUCTURE_LINE_ALTERNATIVE;
+   case DRM_MODE_FLAG_3D_SIDE_BY_SIDE_FULL:
+   return HDMI_3D_STRUCTURE_SIDE_BY_SIDE_FULL;
+   case DRM_MODE_FLAG_3D_L_DEPTH:
+   return HDMI_3D_STRUCTURE_L_DEPTH;
+   case DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH:
+   return HDMI_3D_STRUCTURE_L_DEPTH_GFX_GFX_DEPTH;
+   case DRM_MODE_FLAG_3D_TOP_AND_BOTTOM:
+   return HDMI_3D_STRUCTURE_TOP_AND_BOTTOM;
+   case DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF:
+   return HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF;
+   default:
+   return HDMI_3D_STRUCTURE_INVALID;
+   }
+}
+
 /**
  * drm_hdmi_vendor_infoframe_from_display_mode() - fill an HDMI infoframe with
  * data from a DRM display mode
@@ -3438,20 +3465,29 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct 
hdmi_vendor_infoframe *frame,
const struct drm_display_mode *mode)
 {
int err;
+   u32 s3d_flags;
u8 vic;
 
if (!frame || !mode)
return -EINVAL;
 
vic = drm_match_hdmi_mode(mode);
-   if (!vic)
+   s3d_flags = mode-flags  DRM_MODE_FLAG_3D_MASK;
+
+   if (!vic  !s3d_flags)
+   return -EINVAL;
+
+   if (vic  s3d_flags)
return -EINVAL;
 
err = hdmi_vendor_infoframe_init(frame);
if (err  0)
return err;
 
-   frame-vic = vic;
+   if (vic)
+   frame-vic = vic;
+   else
+   frame-s3d_struct = s3d_structure_from_display_mode(mode);
 
return 0;
 }
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 10/22] drm: Carry over the stereo flags when adding the alternate mode

2013-09-25 Thread Damien Lespiau
This allows to expose the alternate clock versions of the stereo modes.

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/drm_edid.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 48f1746..c24af1d 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2507,6 +2507,9 @@ add_alternate_cea_modes(struct drm_connector *connector, 
struct edid *edid)
if (!newmode)
continue;
 
+   /* Carry over the stereo flags */
+   newmode-flags |= mode-flags  DRM_MODE_FLAG_3D_MASK;
+
/*
 * The current mode could be either variant. Make
 * sure to pick the other clock for the new mode.
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 11/22] drm: Make exposing stereo modes a per-connector opt-in

2013-09-25 Thread Damien Lespiau
Just like with interlaced or double scan modes, make stereo modes a
per-connector opt-in to give a chance to driver authors to make it work
before enabling it.

Suggested-by: Daniel Vetter daniel.vet...@ffwll.ch
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/drm_crtc_helper.c | 8 +++-
 include/drm/drm_crtc.h| 2 ++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c 
b/drivers/gpu/drm/drm_crtc_helper.c
index c722c3b..4280e37 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -76,7 +76,8 @@ static void drm_mode_validate_flag(struct drm_connector 
*connector,
 {
struct drm_display_mode *mode;
 
-   if (flags == (DRM_MODE_FLAG_DBLSCAN | DRM_MODE_FLAG_INTERLACE))
+   if (flags == (DRM_MODE_FLAG_DBLSCAN | DRM_MODE_FLAG_INTERLACE |
+ DRM_MODE_FLAG_3D_MASK))
return;
 
list_for_each_entry(mode, connector-modes, head) {
@@ -86,6 +87,9 @@ static void drm_mode_validate_flag(struct drm_connector 
*connector,
if ((mode-flags  DRM_MODE_FLAG_DBLSCAN) 
!(flags  DRM_MODE_FLAG_DBLSCAN))
mode-status = MODE_NO_DBLESCAN;
+   if ((mode-flags  DRM_MODE_FLAG_3D_MASK) 
+   !(flags  DRM_MODE_FLAG_3D_MASK))
+   mode-status = MODE_NO_STEREO;
}
 
return;
@@ -175,6 +179,8 @@ int drm_helper_probe_single_connector_modes(struct 
drm_connector *connector,
mode_flags |= DRM_MODE_FLAG_INTERLACE;
if (connector-doublescan_allowed)
mode_flags |= DRM_MODE_FLAG_DBLSCAN;
+   if (connector-stereo_allowed)
+   mode_flags |= DRM_MODE_FLAG_3D_MASK;
drm_mode_validate_flag(connector, mode_flags);
 
list_for_each_entry(mode, connector-modes, head) {
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 6b7f9c7..1b69407 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -108,6 +108,7 @@ enum drm_mode_status {
 MODE_ONE_HEIGHT,/* only one height is supported */
 MODE_ONE_SIZE,  /* only one resolution is supported */
 MODE_NO_REDUCED,/* monitor doesn't accept reduced blanking */
+MODE_NO_STEREO,/* stereo modes not supported */
 MODE_UNVERIFIED = -3, /* mode needs to reverified */
 MODE_BAD = -2, /* unspecified reason */
 MODE_ERROR = -1/* error condition */
@@ -611,6 +612,7 @@ struct drm_connector {
int connector_type_id;
bool interlace_allowed;
bool doublescan_allowed;
+   bool stereo_allowed;
struct list_head modes; /* list of modes on this connector */
 
enum drm_connector_status status;
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 15/22] drm: Remove synth_clock from struct drm_display_mode

2013-09-25 Thread Damien Lespiau
This field is unused. Garbage collect it.

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 include/drm/drm_crtc.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 011baaa..8e9716e 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -156,7 +156,6 @@ struct drm_display_mode {
int height_mm;
 
/* Actual mode we give to hw */
-   int synth_clock;
int crtc_hdisplay;
int crtc_hblank_start;
int crtc_hblank_end;
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 12/22] drm: Factor out common CRTC viewport checking code

2013-09-25 Thread Damien Lespiau
Both setcrtc and page_flip are checking that the framebuffer is big
enough for the defined crtc viewport (x, y, hdisplay, vdisplay). Factor
that code out in a single function.

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/drm_crtc.c | 70 --
 1 file changed, 37 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 090415f..db05864 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2063,6 +2063,37 @@ int drm_mode_set_config_internal(struct drm_mode_set 
*set)
 }
 EXPORT_SYMBOL(drm_mode_set_config_internal);
 
+/*
+ * Checks that the framebuffer is big enough for the CRTC viewport
+ * (x, y, hdisplay, vdisplay)
+ */
+static int drm_crtc_check_viewport(const struct drm_crtc *crtc,
+  int x, int y,
+  const struct drm_display_mode *mode,
+  const struct drm_framebuffer *fb)
+
+{
+   int hdisplay, vdisplay;
+
+   hdisplay = mode-hdisplay;
+   vdisplay = mode-vdisplay;
+
+   if (crtc-invert_dimensions)
+   swap(hdisplay, vdisplay);
+
+   if (hdisplay  fb-width ||
+   vdisplay  fb-height ||
+   x  fb-width - hdisplay ||
+   y  fb-height - vdisplay) {
+   DRM_DEBUG_KMS(Invalid fb size %ux%u for CRTC viewport 
%ux%u+%d+%d%s.\n,
+ fb-width, fb-height, hdisplay, vdisplay, x, y,
+ crtc-invert_dimensions ?  (inverted) : );
+   return -ENOSPC;
+   }
+
+   return 0;
+}
+
 /**
  * drm_mode_setcrtc - set CRTC configuration
  * @dev: drm device for the ioctl
@@ -2110,7 +2141,6 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
DRM_DEBUG_KMS([CRTC:%d]\n, crtc-base.id);
 
if (crtc_req-mode_valid) {
-   int hdisplay, vdisplay;
/* If we have a mode we need a framebuffer. */
/* If we pass -1, set the mode with the currently bound fb */
if (crtc_req-fb_id == -1) {
@@ -2146,23 +2176,11 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
 
drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V);
 
-   hdisplay = mode-hdisplay;
-   vdisplay = mode-vdisplay;
-
-   if (crtc-invert_dimensions)
-   swap(hdisplay, vdisplay);
-
-   if (hdisplay  fb-width ||
-   vdisplay  fb-height ||
-   crtc_req-x  fb-width - hdisplay ||
-   crtc_req-y  fb-height - vdisplay) {
-   DRM_DEBUG_KMS(Invalid fb size %ux%u for CRTC viewport 
%ux%u+%d+%d%s.\n,
- fb-width, fb-height,
- hdisplay, vdisplay, crtc_req-x, 
crtc_req-y,
- crtc-invert_dimensions ?  (inverted) : 
);
-   ret = -ENOSPC;
+   ret = drm_crtc_check_viewport(crtc, crtc_req-x, crtc_req-y,
+ mode, fb);
+   if (ret)
goto out;
-   }
+
}
 
if (crtc_req-count_connectors == 0  mode) {
@@ -3579,7 +3597,6 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
struct drm_framebuffer *fb = NULL, *old_fb = NULL;
struct drm_pending_vblank_event *e = NULL;
unsigned long flags;
-   int hdisplay, vdisplay;
int ret = -EINVAL;
 
if (page_flip-flags  ~DRM_MODE_PAGE_FLIP_FLAGS ||
@@ -3611,22 +3628,9 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
if (!fb)
goto out;
 
-   hdisplay = crtc-mode.hdisplay;
-   vdisplay = crtc-mode.vdisplay;
-
-   if (crtc-invert_dimensions)
-   swap(hdisplay, vdisplay);
-
-   if (hdisplay  fb-width ||
-   vdisplay  fb-height ||
-   crtc-x  fb-width - hdisplay ||
-   crtc-y  fb-height - vdisplay) {
-   DRM_DEBUG_KMS(Invalid fb size %ux%u for CRTC viewport 
%ux%u+%d+%d%s.\n,
- fb-width, fb-height, hdisplay, vdisplay, 
crtc-x, crtc-y,
- crtc-invert_dimensions ?  (inverted) : );
-   ret = -ENOSPC;
+   ret = drm_crtc_check_viewport(crtc, crtc-x, crtc-y, crtc-mode, fb);
+   if (ret)
goto out;
-   }
 
if (crtc-fb-pixel_format != fb-pixel_format) {
DRM_DEBUG_KMS(Page flip is not allowed to change frame buffer 
format.\n);
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 13/22] drm: Check the fb size against the adjusted v/hdisplay for stereo modes

2013-09-25 Thread Damien Lespiau
Some stereo modes, like frame packing, need a larger CRTC viewport than
the natural underlying 2D mode and thus drm_crtc_check_viewport()
needs to query the adjusted mode to use the correct h/vdisplay.

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/drm_crtc.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index db05864..807309f 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2078,6 +2078,14 @@ static int drm_crtc_check_viewport(const struct drm_crtc 
*crtc,
hdisplay = mode-hdisplay;
vdisplay = mode-vdisplay;
 
+   if (drm_mode_is_stereo(mode)) {
+   struct drm_display_mode adjusted = *mode;
+
+   drm_mode_set_crtcinfo(adjusted, CRTC_STEREO_DOUBLE);
+   hdisplay = adjusted.crtc_hdisplay;
+   vdisplay = adjusted.crtc_vdisplay;
+   }
+
if (crtc-invert_dimensions)
swap(hdisplay, vdisplay);
 
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 14/22] drm: Remove clock_index from struct drm_display_mode

2013-09-25 Thread Damien Lespiau
This field was only accessed by the nouveau driver, but never set. So
concluded we can rid of this one.

Acked-by: Ben Skeggs bske...@redhat.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/nouveau/dispnv04/crtc.c | 2 --
 include/drm/drm_crtc.h  | 1 -
 2 files changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c 
b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index d4fbf11..0e3270c 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -326,8 +326,6 @@ nv_crtc_mode_set_vga(struct drm_crtc *crtc, struct 
drm_display_mode *mode)
regp-MiscOutReg = 0x23;/* +hsync +vsync */
}
 
-   regp-MiscOutReg |= (mode-clock_index  0x03)  2;
-
/*
 * Time Sequencer
 */
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 1b69407..011baaa 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -156,7 +156,6 @@ struct drm_display_mode {
int height_mm;
 
/* Actual mode we give to hw */
-   int clock_index;
int synth_clock;
int crtc_hdisplay;
int crtc_hblank_start;
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 16/22] drm: Introduce a crtc_clock for struct drm_display_mode

2013-09-25 Thread Damien Lespiau
Just like the various timings, make it possible to have a clock field
what we can tweak before giving it to hardware.

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/drm_modes.c | 1 +
 include/drm/drm_crtc.h  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index c2cb2c8..ef26eb2 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -719,6 +719,7 @@ void drm_mode_set_crtcinfo(struct drm_display_mode *p, int 
adjust_flags)
if ((p == NULL) || ((p-type  DRM_MODE_TYPE_CRTC_C) == 
DRM_MODE_TYPE_BUILTIN))
return;
 
+   p-crtc_clock = p-clock;
p-crtc_hdisplay = p-hdisplay;
p-crtc_hsync_start = p-hsync_start;
p-crtc_hsync_end = p-hsync_end;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 8e9716e..73478bc 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -156,6 +156,7 @@ struct drm_display_mode {
int height_mm;
 
/* Actual mode we give to hw */
+   int crtc_clock; /* in KHz */
int crtc_hdisplay;
int crtc_hblank_start;
int crtc_hblank_end;
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 18/22] drm/i915: Use crtc_clock in intel_dump_crtc_timings()

2013-09-25 Thread Damien Lespiau
We want to dump the parameters given to the hardware, so let's use
crtc_clock here.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index d12d563..88c641a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8310,7 +8310,7 @@ static void intel_dump_crtc_timings(const struct 
drm_display_mode *mode)
 {
DRM_DEBUG_KMS(crtc timings: %d %d %d %d %d %d %d %d %d, 
type: 0x%x flags: 0x%x\n,
-   mode-clock,
+   mode-crtc_clock,
mode-crtc_hdisplay, mode-crtc_hsync_start,
mode-crtc_hsync_end, mode-crtc_htotal,
mode-crtc_vdisplay, mode-crtc_vsync_start,
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 17/22] drm: Implement timings adjustments for frame packing

2013-09-25 Thread Damien Lespiau
When using the frame packing and a single big framebuffer, some hardware
requires that we do everything like if we were scanning out the big
buffer itself. Let's instrument drm_mode_set_crtcinfo() to be able to do
this adjustement if the driver is asking for it.

v2: Use crtc_vtotal and multiply the clock by 2 instead of
reconstructing it (Ville Syrjälä)

Suggested-by: Daniel Vetter daniel.vet...@ffwll.ch
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/drm_modes.c | 22 +-
 include/drm/drm_crtc.h  |  3 ++-
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index ef26eb2..b073315 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -707,12 +707,18 @@ EXPORT_SYMBOL(drm_mode_vrefresh);
 /**
  * drm_mode_set_crtcinfo - set CRTC modesetting parameters
  * @p: mode
- * @adjust_flags: unused? (FIXME)
+ * @adjust_flags: a combination of adjustment flags
  *
  * LOCKING:
  * None.
  *
  * Setup the CRTC modesetting parameters for @p, adjusting if necessary.
+ *
+ * - The CRTC_INTERLACE_HALVE_V flag can be used to halve vertical timings of
+ *   interlaced modes.
+ * - The CRTC_STEREO_DOUBLE flag can be used to compute the timings for
+ *   buffers containing two eyes (only adjust the timings when needed, eg. for
+ *   frame packing or side by side full).
  */
 void drm_mode_set_crtcinfo(struct drm_display_mode *p, int adjust_flags)
 {
@@ -753,6 +759,20 @@ void drm_mode_set_crtcinfo(struct drm_display_mode *p, int 
adjust_flags)
p-crtc_vtotal *= p-vscan;
}
 
+   if (adjust_flags  CRTC_STEREO_DOUBLE) {
+   unsigned int layout = p-flags  DRM_MODE_FLAG_3D_MASK;
+
+   switch (layout) {
+   case DRM_MODE_FLAG_3D_FRAME_PACKING:
+   p-crtc_clock *= 2;
+   p-crtc_vdisplay += p-crtc_vtotal;
+   p-crtc_vsync_start += p-crtc_vtotal;
+   p-crtc_vsync_end += p-crtc_vtotal;
+   p-crtc_vtotal += p-crtc_vtotal;
+   break;
+   }
+   }
+
p-crtc_vblank_start = min(p-crtc_vsync_start, p-crtc_vdisplay);
p-crtc_vblank_end = max(p-crtc_vsync_end, p-crtc_vtotal);
p-crtc_hblank_start = min(p-crtc_hsync_start, p-crtc_hdisplay);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 73478bc..b2d08ca 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -125,7 +125,8 @@ enum drm_mode_status {
.vscan = (vs), .flags = (f), \
.base.type = DRM_MODE_OBJECT_MODE
 
-#define CRTC_INTERLACE_HALVE_V 0x1 /* halve V values for interlacing */
+#define CRTC_INTERLACE_HALVE_V (1  0) /* halve V values for interlacing */
+#define CRTC_STEREO_DOUBLE (1  1) /* adjust timings for stereo modes */
 
 struct drm_display_mode {
/* Header */
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 19/22] drm/i915: Use crtc_clock with the adjusted mode

2013-09-25 Thread Damien Lespiau
struct drm_mode_display now has a separate crtc_ version of the clock to
be used when we're talking about the timings given to the harwadre (was
far as the mode is concerned).

This commit is really the result of a git grep adjusted_mode.*clock and
replacing those by adjusted_mode.crtc_clock. No functional change.

v2: Rebased on drm-intel-queued-next

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/i915/intel_crt.c |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 34 +-
 drivers/gpu/drm/i915/intel_dp.c  | 11 +++
 drivers/gpu/drm/i915/intel_drv.h |  2 +-
 drivers/gpu/drm/i915/intel_dvo.c |  2 +-
 drivers/gpu/drm/i915/intel_hdmi.c|  6 +++---
 drivers/gpu/drm/i915/intel_lvds.c|  2 +-
 drivers/gpu/drm/i915/intel_pm.c  | 36 +++-
 drivers/gpu/drm/i915/intel_sdvo.c|  2 +-
 drivers/gpu/drm/i915/intel_tv.c  |  2 +-
 10 files changed, 56 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 019c4ce..0263629 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -117,7 +117,7 @@ static void intel_crt_get_config(struct intel_encoder 
*encoder,
if (HAS_PCH_SPLIT(dev))
ironlake_check_encoder_dotclock(pipe_config, dotclock);
 
-   pipe_config-adjusted_mode.clock = dotclock;
+   pipe_config-adjusted_mode.crtc_clock = dotclock;
 }
 
 static void hsw_crt_get_config(struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 88c641a..3c982a4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -739,14 +739,14 @@ bool intel_crtc_active(struct drm_crtc *crtc)
/* Be paranoid as we can arrive here with only partial
 * state retrieved from the hardware during setup.
 *
-* We can ditch the adjusted_mode.clock check as soon
+* We can ditch the adjusted_mode.crtc_clock check as soon
 * as Haswell has gained clock readout/fastboot support.
 *
 * We can ditch the crtc-fb check as soon as we can
 * properly reconstruct framebuffers.
 */
return intel_crtc-active  crtc-fb 
-   intel_crtc-config.adjusted_mode.clock;
+   intel_crtc-config.adjusted_mode.crtc_clock;
 }
 
 enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv,
@@ -2913,7 +2913,7 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
 {
struct drm_device *dev = crtc-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
-   int clock = to_intel_crtc(crtc)-config.adjusted_mode.clock;
+   int clock = to_intel_crtc(crtc)-config.adjusted_mode.crtc_clock;
u32 divsel, phaseinc, auxdiv, phasedir = 0;
u32 temp;
 
@@ -2937,8 +2937,8 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
phaseinc = 0x20;
} else {
/* The iCLK virtual clock root frequency is in MHz,
-* but the adjusted_mode-clock in in KHz. To get the divisors,
-* it is necessary to divide one by another, so we
+* but the adjusted_mode-crtc_clock in in KHz. To get the
+* divisors, it is necessary to divide one by another, so we
 * convert the virtual clock precision to KHz here for higher
 * precision.
 */
@@ -4148,7 +4148,7 @@ retry:
 */
link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10;
 
-   fdi_dotclock = adjusted_mode-clock;
+   fdi_dotclock = adjusted_mode-crtc_clock;
 
lane = ironlake_get_lanes_required(fdi_dotclock, link_bw,
   pipe_config-pipe_bpp);
@@ -4204,12 +4204,12 @@ static int intel_crtc_compute_config(struct intel_crtc 
*crtc,
 * otherwise pipe A only.
 */
if ((crtc-pipe == PIPE_A || IS_I915G(dev)) 
-   adjusted_mode-clock  clock_limit * 9 / 10) {
+   adjusted_mode-crtc_clock  clock_limit * 9 / 10) {
clock_limit *= 2;
pipe_config-double_wide = true;
}
 
-   if (adjusted_mode-clock  clock_limit * 9 / 10)
+   if (adjusted_mode-crtc_clock  clock_limit * 9 / 10)
return -EINVAL;
}
 
@@ -4869,7 +4869,7 @@ static void intel_crtc_mode_from_pipe_config(struct 
intel_crtc *intel_crtc,
 
crtc-mode.flags = pipe_config-adjusted_mode.flags;
 
-   crtc-mode.clock = pipe_config-adjusted_mode.clock;
+   crtc-mode.clock = pipe_config-adjusted_mode.crtc_clock;
crtc-mode.flags |= pipe_config-adjusted_mode.flags;
 }
 
@@ -7440,7 +7440,7 @@ static void 

[Intel-gfx] [PATCH 20/22] drm/i915: Ask the DRM core do make stereo timings adjustements

2013-09-25 Thread Damien Lespiau
When scanning out big stereo buffers that are actually bigger that their
natural 2D counterpart, we need to blow up the crtc timings as well.

Not that this is only done for frame packing as this is the only stereo
mode currently exposed needing this kind of ajdustements.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 3c982a4..c25622d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8435,7 +8435,7 @@ encoder_retry:
pipe_config-pixel_multiplier = 1;
 
/* Fill in default crtc timings, allow encoders to overwrite them. */
-   drm_mode_set_crtcinfo(pipe_config-adjusted_mode, 0);
+   drm_mode_set_crtcinfo(pipe_config-adjusted_mode, CRTC_STEREO_DOUBLE);
 
/* Pass our mode to the connectors and the CRTC to give them a chance to
 * adjust it according to limitations or connector properties, and also
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 22/22] drm/i915: Allow stereo modes on HDMI

2013-09-25 Thread Damien Lespiau
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/i915/intel_hdmi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index 1a57758..6004f9c 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1228,6 +1228,7 @@ void intel_hdmi_init_connector(struct intel_digital_port 
*intel_dig_port,
 
connector-interlace_allowed = 1;
connector-doublescan_allowed = 0;
+   connector-stereo_allowed = 1;
 
switch (port) {
case PORT_B:
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 21/22] drm/i915: Prefer crtc_{h|v}display for pipe src dimensions

2013-09-25 Thread Damien Lespiau
Now that we ask to adjust the crtc timings for stereo modes, the correct
pipe_src_w and pipe_src_h can be found in crtc_vdisplay and crtc_hdisplay.

v2: Add comment about why pipe_src_w/h need to be set afert
set_crtcinfo() (Daniel Vetter)

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

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index c25622d..c94fe38 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8400,9 +8400,6 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
drm_mode_copy(pipe_config-adjusted_mode, mode);
drm_mode_copy(pipe_config-requested_mode, mode);
 
-   pipe_config-pipe_src_w = mode-hdisplay;
-   pipe_config-pipe_src_h = mode-vdisplay;
-
pipe_config-cpu_transcoder =
(enum transcoder) to_intel_crtc(crtc)-pipe;
pipe_config-shared_dpll = DPLL_ID_PRIVATE;
@@ -8437,6 +8434,10 @@ encoder_retry:
/* Fill in default crtc timings, allow encoders to overwrite them. */
drm_mode_set_crtcinfo(pipe_config-adjusted_mode, CRTC_STEREO_DOUBLE);
 
+   /* set_crtcinfo() may have adjusted hdisplay/vdisplay */
+   pipe_config-pipe_src_w = pipe_config-adjusted_mode.crtc_hdisplay;
+   pipe_config-pipe_src_h = pipe_config-adjusted_mode.crtc_vdisplay;
+
/* Pass our mode to the connectors and the CRTC to give them a chance to
 * adjust it according to limitations or connector properties, and also
 * a chance to reject the mode entirely.
-- 
1.8.3.1

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


Re: [Intel-gfx] [PATCH v3 0/4] Fix Win8 backlight issue

2013-09-25 Thread Jörg Otte
2013/9/25 Jani Nikula jani.nik...@linux.intel.com:
 On Wed, 25 Sep 2013, Aaron Lu aaron...@intel.com wrote:
 On Wed, Sep 25, 2013 at 10:29:37AM +0200, Jörg Otte wrote:
 Backlight can't be modified with this patch set - neither with
 function keys nor with the gui. It is a step backward to v3.11-rc1

 So both hotkeys and GUI work in v3.11-rc1? And v3.12-rc2?

In  v3.11-rc1 it didn't work.
Since a later rc (I don't remember exactly which) it did work.
In v3.12-rc1/2 it does work.
In v3.12-rc2 + Aaron's patch set it again doesn't work.


 Thanks for the test.

 Please check /sys/class/backlight, is there only one link file
 intel_backlight?

 Indeed, are there others? fujitsu-laptop perhaps? If yes, try
 CONFIG_FUJITSU_LAPTOP=n or an appropriate module blacklist.

 Just checking, you do have CONFIG_BACKLIGHT_CLASS_DEVICE=y?

There is only one intel_backlight link.
Yes, I have CONFIG_BACKLIGHT_CLASS_DEVICE=y

 Embarrassingly there was a bug in i915 fixed just recently where the
 backlight device wasn't registered for
 CONFIG_BACKLIGHT_CLASS_DEVICE=m...

 If so, please cd inside and try modify the brightness file using echo
 with some values in the range of 0 - max_brightness, does the
 backlight level change when you echo a new value? I guess it doesn't,
 or you wouldn't notice problem. If indeed so, perhaps file a bug at
 http://bugzilla.kernel.org, Drivers/Video(DRI-Intel)? I suppose Jani
 and Daniel can help fix your problem.

 Yup, please check the above, and report back.

- echo 0..max_brightness  brightness does not work.


 Video driver: i915
 FUJITSU LIFEBOOK AH532/FJNBB1C, BIOS Version 1.09 05/22/2012

 acpi_backlight=video works.

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


Re: [Intel-gfx] [PATCH v3 0/4] Fix Win8 backlight issue

2013-09-25 Thread Jani Nikula
On Wed, 25 Sep 2013, Jörg Otte jrg.o...@gmail.com wrote:
 2013/9/25 Jani Nikula jani.nik...@linux.intel.com:
 On Wed, 25 Sep 2013, Aaron Lu aaron...@intel.com wrote:
 On Wed, Sep 25, 2013 at 10:29:37AM +0200, Jörg Otte wrote:
 Backlight can't be modified with this patch set - neither with
 function keys nor with the gui. It is a step backward to v3.11-rc1

 So both hotkeys and GUI work in v3.11-rc1? And v3.12-rc2?

 In  v3.11-rc1 it didn't work.
 Since a later rc (I don't remember exactly which) it did work.
 In v3.12-rc1/2 it does work.
 In v3.12-rc2 + Aaron's patch set it again doesn't work.


 Thanks for the test.

 Please check /sys/class/backlight, is there only one link file
 intel_backlight?

 Indeed, are there others? fujitsu-laptop perhaps? If yes, try
 CONFIG_FUJITSU_LAPTOP=n or an appropriate module blacklist.

 Just checking, you do have CONFIG_BACKLIGHT_CLASS_DEVICE=y?

 There is only one intel_backlight link.
 Yes, I have CONFIG_BACKLIGHT_CLASS_DEVICE=y

 Embarrassingly there was a bug in i915 fixed just recently where the
 backlight device wasn't registered for
 CONFIG_BACKLIGHT_CLASS_DEVICE=m...

 If so, please cd inside and try modify the brightness file using echo
 with some values in the range of 0 - max_brightness, does the
 backlight level change when you echo a new value? I guess it doesn't,
 or you wouldn't notice problem. If indeed so, perhaps file a bug at
 http://bugzilla.kernel.org, Drivers/Video(DRI-Intel)? I suppose Jani
 and Daniel can help fix your problem.

 Yup, please check the above, and report back.

 - echo 0..max_brightness  brightness does not work.

Thanks for the info.

How about v3.12-rc2 without Aaron's patches? Does intel_backlight also
not work there? How about the acpi_video0 (which I presume is present)
sysfs interface?

BR,
Jani.





 Video driver: i915
 FUJITSU LIFEBOOK AH532/FJNBB1C, BIOS Version 1.09 05/22/2012

 acpi_backlight=video works.

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


[Intel-gfx] [PATCH] drm/i915/vlv: hack to init backlight regs if BIOS fails to do so

2013-09-25 Thread Jesse Barnes
Still digging up the actual VBT info for this, but wanted to get this
out there for testing, or in case others are also bugged by this.

This can happen if you boot with an external display connected.  In that
case, the attached eDP backlight modulation frequency may not be
programmed, so we need to use something (in this case the value my BIOS
normally programs with just the internal display enabled).

Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
 drivers/gpu/drm/i915/intel_panel.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_panel.c 
b/drivers/gpu/drm/i915/intel_panel.c
index 3bc89a6..a3536785 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -372,6 +372,9 @@ static u32 i915_read_blc_pwm_ctl(struct drm_device *dev)
I915_WRITE(BLC_PWM_CTL2,
   dev_priv-regfile.saveBLC_PWM_CTL2);
}
+
+   if (IS_VALLEYVIEW(dev)  !val)
+   val = 0x;
}
 
return val;
@@ -629,10 +632,24 @@ set_level:
spin_unlock_irqrestore(dev_priv-backlight.lock, flags);
 }
 
+/* FIXME: use VBT vals to init PWM_CTL and PWM_CTL2 correctly */
+static void intel_panel_init_backlight_regs(struct drm_device *dev)
+{
+   struct drm_i915_private *dev_priv = dev-dev_private;
+
+   if (IS_VALLEYVIEW(dev)) {
+   u32 cur_val = I915_READ(BLC_PWM_CTL) 
+   ~BACKLIGHT_DUTY_CYCLE_MASK;
+   I915_WRITE(BLC_PWM_CTL, (0xf42  16) | cur_val);
+   }
+}
+
 static void intel_panel_init_backlight(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
 
+   intel_panel_init_backlight_regs(dev);
+
dev_priv-backlight.level = intel_panel_get_backlight(dev);
dev_priv-backlight.enabled = dev_priv-backlight.level != 0;
 }
-- 
1.8.3.1

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


[Intel-gfx] Upstreaming the stereo v6 series

2013-09-25 Thread Damien Lespiau
Hi,

So this series looks like a good candidate to be merged in one tree.

Beside the new 3d flags added to the mode structure, the other new API
is the SET_CLIENT_CAP ioctl. It seems that this new ioctl could already
be potentially useful for user space to tell us they want the primary
plane explosed as a DRM plane.

The i915 bits depend on the lastest drm-intel(-next-queued) so it'd be
simpler to merge this series in drm-intel rather than drm-next. Options
are:

  - merge it through drm-intel (yey!)

  - merge it through drm-next once the current drm-intel has been merged
(will probably need a rebase because of the crtc_clock addition)

  - merge the drm patches through drm-next and the drm/i915 ones through
drm-intel, but that'll likely need me to rebase the i915 patches as
well.

All in all, it'd be much easier to merge it through drm-intel (if people
are happy with the current state of the series, of course).

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


[Intel-gfx] [PATCH] drm/i915: Fix pre-CTG vblank counter

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

The old style frame counter increments at the start of active video.
However for i915_get_vblank_counter() we want a counter that increments
at the start of vblank.

Fortunately the low frame counter register also contains the pixel
counter for the current frame. We can can compare that against the
vblank start pixel count to determine if we need to increment the
frame counter by 1 to get the correct answer.

Also reorganize the function pointer assignments in intel_irq_init() a
bit to avoid confusing people.

Cc: Mario Kleiner mario.klei...@tuebingen.mpg.de
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---

Just another small vblank related gem I forgot to polish up and send
out until Imre started asking me questions about the vblank counter
functions.

 drivers/gpu/drm/i915/i915_irq.c | 36 +++-
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 4b519c8..8a5eb9d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -526,7 +526,7 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, 
int pipe)
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev-dev_private;
unsigned long high_frame;
unsigned long low_frame;
-   u32 high1, high2, low;
+   u32 high1, high2, low, pixel, vbl_start;
 
if (!i915_pipe_enabled(dev, pipe)) {
DRM_DEBUG_DRIVER(trying to get vblank count for disabled 
@@ -534,6 +534,24 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, 
int pipe)
return 0;
}
 
+   if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+   struct intel_crtc *intel_crtc =
+   to_intel_crtc(dev_priv-pipe_to_crtc_mapping[pipe]);
+   const struct drm_display_mode *mode =
+   intel_crtc-config.adjusted_mode;
+
+   vbl_start = mode-crtc_vblank_start * mode-crtc_htotal;
+   } else {
+   enum transcoder cpu_transcoder =
+   intel_pipe_to_cpu_transcoder(dev_priv, pipe);
+   u32 htotal;
+
+   htotal = ((I915_READ(HTOTAL(cpu_transcoder))  16)  0x1fff) + 
1;
+   vbl_start = (I915_READ(VBLANK(cpu_transcoder))  0x1fff) + 1;
+
+   vbl_start *= htotal;
+   }
+
high_frame = PIPEFRAME(pipe);
low_frame = PIPEFRAMEPIXEL(pipe);
 
@@ -544,13 +562,20 @@ static u32 i915_get_vblank_counter(struct drm_device 
*dev, int pipe)
 */
do {
high1 = I915_READ(high_frame)  PIPE_FRAME_HIGH_MASK;
-   low   = I915_READ(low_frame)   PIPE_FRAME_LOW_MASK;
+   low   = I915_READ(low_frame);
high2 = I915_READ(high_frame)  PIPE_FRAME_HIGH_MASK;
} while (high1 != high2);
 
high1 = PIPE_FRAME_HIGH_SHIFT;
+   pixel = low  PIPE_PIXEL_MASK;
low = PIPE_FRAME_LOW_SHIFT;
-   return (high1  8) | low;
+
+   /*
+* The frame counter increments at beginning of active.
+* Cook up a vblank counter by also checking the pixel
+* counter against vblank start.
+*/
+   return ((high1  8) | low) + (pixel = vbl_start);
 }
 
 static u32 gm45_get_vblank_counter(struct drm_device *dev, int pipe)
@@ -3197,11 +3222,12 @@ void intel_irq_init(struct drm_device *dev)
 
pm_qos_add_request(dev_priv-pm_qos, PM_QOS_CPU_DMA_LATENCY, 
PM_QOS_DEFAULT_VALUE);
 
-   dev-driver-get_vblank_counter = i915_get_vblank_counter;
-   dev-max_vblank_count = 0xff; /* only 24 bits of frame count */
if (IS_G4X(dev) || INTEL_INFO(dev)-gen = 5) {
dev-max_vblank_count = 0x; /* full 32 bit counter */
dev-driver-get_vblank_counter = gm45_get_vblank_counter;
+   } else {
+   dev-driver-get_vblank_counter = i915_get_vblank_counter;
+   dev-max_vblank_count = 0xff; /* only 24 bits of frame 
count */
}
 
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
-- 
1.8.1.5

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


[Intel-gfx] [PATCH 2/3] drm/i915: Boost RPS frequency for CPU stalls

2013-09-25 Thread Chris Wilson
If we encounter a situation where the CPU blocks waiting for results
from the GPU, give the GPU a kick to boost its the frequency.

This should work to reduce user interface stalls and to quickly promote
mesa to high frequencies - but the cost is that our requested frequency
stalls high (as we do not idle for long enough before rc6 to start
reducing frequencies, nor are we aggressive at down clocking an
underused GPU). However, this should be mitigated by rc6 itself powering
off the GPU when idle, and that energy use is dependent upon the workload
of the GPU in addition to its frequency (e.g. the math or sampler
functions only consume power when used). Still, this is likely to
adversely affect light workloads.

In particular, this nearly eliminates the highly noticeable wake-up lag
in animations from idle. For example, expose or workspace transitions.
(However, given the situation where we fail to downclock, our requested
frequency is almost always the maximum, except for Baytrail where we
manually downclock upon idling. This often masks the latency of
upclocking after being idle, so animations are typically smooth - at the
cost of increased power consumption.)

Stéphane raised the concern that this will punish good applications and
reward bad applications - but due to the nature of how mesa performs its
client throttling, I believe all mesa applications will be roughly
equally affected. To address this concern, and to prevent applications
like compositors from permanently boosting the RPS state, we ratelimit the
frequency of the wait-boosts each client recieves.

Unfortunately, this techinique is ineffective with Ironlake - which also
has dynamic render power states and suffers just as dramatically. For
Ironlake, the thermal/power headroom is shared with the CPU through
Intelligent Power Sharing and the intel-ips module. This leaves us with
no GPU boost frequencies available when coming out of idle, and due to
hardware limitations we cannot change the arbitration between the CPU and
GPU quickly enough to be effective.

v2: Limit each client to receiving a single boost for each active period.
Tested by QA to only marginally increase power, and to demonstrably
increase throughput in games. No latency measurements yet.

v3: Cater for front-buffer rendering with manual throttling.

v4: Tidy up.

v5: Sadly the compositor needs frequent boosts as it may never idle, but
due to its picking mechanism (using ReadPixels) may require frequent
waits. Those waits, along with the waits for the vrefresh swap, conspire
to keep the GPU at low frequencies despite the interactive latency. To
overcome this we ditch the one-boost-per-active-period and just ratelimit
the number of wait-boosts each client can receive.

Reported-and-tested-by: Paul Neumann paul1...@yahoo.de
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=68716
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Kenneth Graunke kenn...@whitecape.org
Cc: Stéphane Marchesin stephane.marche...@gmail.com
Cc: Owen Taylor otay...@redhat.com
Cc: Meng, Mengmeng mengmeng.m...@intel.com
Cc: Zhuang, Lena lena.zhu...@intel.com
---
 drivers/gpu/drm/i915/i915_dma.c  |  16 ++---
 drivers/gpu/drm/i915/i915_drv.h  |  19 +++--
 drivers/gpu/drm/i915/i915_gem.c  | 135 ---
 drivers/gpu/drm/i915/i915_irq.c  |  11 ---
 drivers/gpu/drm/i915/intel_display.c |   3 +
 drivers/gpu/drm/i915/intel_drv.h |   3 +
 drivers/gpu/drm/i915/intel_pm.c  |  42 ++-
 7 files changed, 138 insertions(+), 91 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index d35de1b..57ca5af 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1816,19 +1816,11 @@ int i915_driver_unload(struct drm_device *dev)
 
 int i915_driver_open(struct drm_device *dev, struct drm_file *file)
 {
-   struct drm_i915_file_private *file_priv;
-
-   DRM_DEBUG_DRIVER(\n);
-   file_priv = kzalloc(sizeof(*file_priv), GFP_KERNEL);
-   if (!file_priv)
-   return -ENOMEM;
-
-   file-driver_priv = file_priv;
-
-   spin_lock_init(file_priv-mm.lock);
-   INIT_LIST_HEAD(file_priv-mm.request_list);
+   int ret;
 
-   idr_init(file_priv-context_idr);
+   ret = i915_gem_open(dev, file);
+   if (ret)
+   return ret;
 
return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 906f4fb..ddc528b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -853,9 +853,6 @@ struct intel_gen6_power_mgmt {
struct work_struct work;
u32 pm_iir;
 
-   /* On vlv we need to manually drop to Vmin with a delayed work. */
-   struct delayed_work vlv_work;
-
/* The below variables an all the rps hw state are protected by
 * dev-struct mutext. */
u8 cur_delay;
@@ -974,6 +971,15 @@ struct i915_gem_mm {
struct 

Re: [Intel-gfx] [PATCH] drm/i915: we can't call move_to_active before intel_ring_begin

2013-09-25 Thread Ben Widawsky
On Wed, Sep 25, 2013 at 05:05:15PM +0200, Daniel Vetter wrote:
 Otherwise we can die in a fire of not-yet-allocated lazy requests when
 we expect them to be there:

And Chris accuses me of violating Rusty's rules. This is an extremely
ugly caveat to put in an interface given that active tracking and ring
dispatch should have little connection.


Isn't it much simpler to just call intel_ring_alloc_seqno during
move_to_active?

 
 [ 4405.463113] [ cut here ]
 [ 4405.464769] kernel BUG at 
 /home/user/src/linux/drivers/gpu/drm/i915/intel_ringbuffer.h:268!
 [ 4405.466392] invalid opcode:  [#1] PREEMPT SMP
 [ 4405.468007] Modules linked in: snd_hda_codec_hdmi hid_generic 
 snd_hda_codec_realtek usbhid hid iTCO_wdt snd_hda_intel psmouse i2c_i801 
 snd_hda_codec pcspkr snd_hwdep i915 snd_pcm snd_page_alloc snd_timer 
 cfbfillrect snd cfbimgblt lpc_ich cfbcopyarea soundcore drm_kms_helper 
 mfd_core evdev wmi
 [ 4405.471387] CPU: 5 PID: 9300 Comm: gem_storedw_bat Not tainted 
 3.12.0-rc2-drm_vbl+ #1
 [ 4405.473047] Hardware name:  /DZ77BH-55K, BIOS 
 BHZ7710H.86A.0100.2013.0517.0942 05/17/2013
 [ 4405.474712] task: 8800618d4b00 ti: 88010a806000 task.ti: 
 88010a806000
 [ 4405.476370] RIP: 0010:[a009ffa9]  [a009ffa9] 
 i915_vma_move_to_active+0x1b9/0x1e0 [i915]
 [ 4405.478045] RSP: 0018:88010a807be8  EFLAGS: 00010246
 [ 4405.479689] RAX: 88011a681000 RBX: 8800b364f9c0 RCX: 
 88011902b898
 [ 4405.481321] RDX: 8800d4a1b6e0 RSI: 8800d4a1a8b8 RDI: 
 88011902b840
 [ 4405.482932] RBP: 88010a807c08 R08: 0001 R09: 
 
 [ 4405.484526] R10: 0001 R11:  R12: 
 8800d4a1a8b8
 [ 4405.486100] R13:  R14: 8800d4a18000 R15: 
 8800b364f9c0
 [ 4405.487664] FS:  7f36c1a738c0() GS:88011f34() 
 knlGS:
 [ 4405.489216] CS:  0010 DS:  ES:  CR0: 80050033
 [ 4405.490747] CR2: 7fff1b28ea30 CR3: 000119e0d000 CR4: 
 001407e0
 [ 4405.492276] Stack:
 [ 4405.493774]  88010a807dd8 8800d4a1a8b8 8800d3c1c400 
 a00ac060
 [ 4405.495276]  88010a807d28 a00aa0db 88010a807cb8 
 810aa4e4
 [ 4405.496776]  0003 8801 8800618d50e8 
 81a9da00
 [ 4405.498265] Call Trace:
 [ 4405.499735]  [a00ac060] ? gen6_ppgtt_insert_entries+0x1a0/0x1a0 
 [i915]
 [ 4405.501218]  [a00aa0db] 
 i915_gem_do_execbuffer.clone.18+0x65b/0x12e0 [i915]
 [ 4405.502685]  [810aa4e4] ? lock_release_non_nested+0xa4/0x360
 [ 4405.504134]  [a00ab298] i915_gem_execbuffer2+0xa8/0x290 [i915]
 [ 4405.505573]  [813552b9] drm_ioctl+0x419/0x5c0
 [ 4405.506991]  [81128b12] ? handle_mm_fault+0x352/0xa00
 [ 4405.508399]  [a00ab1f0] ? i915_gem_execbuffer+0x490/0x490 [i915]
 [ 4405.509792]  [8103cd2c] ? __do_page_fault+0x1fc/0x4b0
 [ 4405.511170]  [81165e66] do_vfs_ioctl+0x96/0x560
 [ 4405.512533]  [81512163] ? error_sti+0x5/0x6
 [ 4405.513878]  [81511d0d] ? retint_swapgs+0xe/0x13
 [ 4405.515208]  [811663d1] SyS_ioctl+0xa1/0xb0
 [ 4405.516522]  [8129ddee] ? trace_hardirqs_on_thunk+0x3a/0x3f
 [ 4405.517830]  [81512792] system_call_fastpath+0x16/0x1b
 [ 4405.519119] Code: f6 83 40 01 00 00 01 0f 85 11 ff ff ff b8 01 00 00 00 f0 
 0f c1 03 ff c0 83 f8 01 7e 10 80 8b 40 01 00 00 01 e9 f5 fe ff ff 0f 0b 0f 
 0b 80 3d 61 55 08 00 01 74 e7 be 2f 00 00 00 48 c7 c7 18 03
 [ 4405.520610] RIP  [a009ffa9] i915_vma_move_to_active+0x1b9/0x1e0 
 [i915]
 [ 4405.522001]  RSP 88010a807be8
 [ 4405.523367] [drm:intel_crtc_set_config], [CRTC:3] [FB:50] #connectors=1 (x 
 y) (0 0)
 [ 4405.524751] [drm:intel_set_config_compute_mode_changes], computed changes 
 for [CRTC:3], mode_changed=0, fb_changed=0

 [ 4405.526152] [drm:intel_modeset_stage_output_state], 
 [CONNECTOR:16:HDMI-A-2] to [CRTC:3]
 [ 4405.531096] [drm:intel_crtc_set_config], [CRTC:3] [FB:50] #connectors=1 (x 
 y) (0 0)
 [ 4405.532491] [drm:intel_set_config_compute_mode_changes], computed changes 
 for [CRTC:3], mode_changed=0, fb_changed=0
 [ 4405.533903] [drm:intel_modeset_stage_output_state], 
 [CONNECTOR:16:HDMI-A-2] to [CRTC:3]
 [ 4405.53] ---[ end trace 53d1b708421bb5b3 ]---
 
 This regression has been introduced in from Ben Widawsky's ppgtt/vma
 enabling patch drm/i915: Use the new vm [un]bind functions.
 
 This should be exercised by
 igt/gem_storedw_batches_loop/secure-dispatch.
 
 Note that this won't fix the full ordeal for real ppgtt since the
 potential allocation for the batch vma could recurse into the shrinker
 and wreak utter havoc with our carefully reserved buffers. But for now
 this is good enough.
 
 The real fix involves some trickery to allocate the batch vma around
 the call to eb_lookup_vmas and do all the additional buffer space
 reserving for the batch in global gtt in the normal 

[Intel-gfx] [PATCH 3/3] drm/i915: Tweak RPS thresholds to more aggressively downclock

2013-09-25 Thread Chris Wilson
After applying wait-boost we often find ourselves stuck at higher clocks
than required. The current threshold value requires the GPU to be
continuously and completely idle for 313ms before it is dropped by one
bin. Conversely, we require the GPU to be busy for an average of 90% over
a 84ms period before we upclock. So the current thresholds almost never
downclock the GPU, and respond very slowly to sudden demands for more
power. It is easy to observe that we currently lock into the wrong bin
and both underperform in benchmarks and consume more power than optimal
(just by repeating the task and measuring the different results).

An alternative approach, as discussed in the bspec, is to use a
continuous threshold for upclocking, and an average value for downclocking.
This is good for quickly detecting and reacting to state changes within a
frame, however it fails with the common throttling method of waiting
upon the outstanding frame - at least it is difficult to choose a
threshold that works well at 15,000fps and at 60fps. So continue to use
average busy/idle loads to determine frequency change.

v2: Use 3 power zones to keep frequencies low in steady-state mostly
idle (e.g. scrolling, interactive 2D drawing), and frequencies high
for demanding games. In between those end-states, we use a
fast-reclocking algorithm to converge more quickly on the desired bin.

v3: Bug fixes - make sure we reset adj after switching power zones.

v4: Tune - drop the continuous busy thresholds as it prevents us from
choosing the right frequency for glxgears style swap benchmarks. Instead
the goal is to be able to find the right clocks irrespective of the
wait-boost.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Kenneth Graunke kenn...@whitecape.org
Cc: Stéphane Marchesin stephane.marche...@gmail.com
Cc: Owen Taylor otay...@redhat.com
Cc: Meng, Mengmeng mengmeng.m...@intel.com
Cc: Zhuang, Lena lena.zhu...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h |   5 ++
 drivers/gpu/drm/i915/i915_irq.c |  46 ++
 drivers/gpu/drm/i915/i915_reg.h |   2 +-
 drivers/gpu/drm/i915/intel_pm.c | 137 ++--
 4 files changed, 143 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ddc528b..fd0a28d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -859,8 +859,13 @@ struct intel_gen6_power_mgmt {
u8 min_delay;
u8 max_delay;
u8 rpe_delay;
+   u8 rp1_delay;
+   u8 rp0_delay;
u8 hw_max;
 
+   int last_adj;
+   enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
+
struct delayed_work delayed_resume_work;
 
/*
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 6ee5572..418ad64 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -818,7 +818,7 @@ static void gen6_pm_rps_work(struct work_struct *work)
drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t,
rps.work);
u32 pm_iir;
-   u8 new_delay;
+   int new_delay, adj;
 
spin_lock_irq(dev_priv-irq_lock);
pm_iir = dev_priv-rps.pm_iir;
@@ -835,29 +835,49 @@ static void gen6_pm_rps_work(struct work_struct *work)
 
mutex_lock(dev_priv-rps.hw_lock);
 
+   adj = dev_priv-rps.last_adj;
if (pm_iir  GEN6_PM_RP_UP_THRESHOLD) {
-   new_delay = dev_priv-rps.cur_delay + 1;
+   if (adj  0)
+   adj *= 2;
+   else
+   adj = 1;
+   new_delay = dev_priv-rps.cur_delay + adj;
 
/*
 * For better performance, jump directly
 * to RPe if we're below it.
 */
-   if (IS_VALLEYVIEW(dev_priv-dev) 
-   dev_priv-rps.cur_delay  dev_priv-rps.rpe_delay)
+   if (new_delay  dev_priv-rps.rpe_delay)
+   new_delay = dev_priv-rps.rpe_delay;
+   } else if (pm_iir  GEN6_PM_RP_DOWN_TIMEOUT) {
+   if (dev_priv-rps.cur_delay  dev_priv-rps.rpe_delay)
new_delay = dev_priv-rps.rpe_delay;
-   } else
-   new_delay = dev_priv-rps.cur_delay - 1;
+   else
+   new_delay = dev_priv-rps.min_delay;
+   adj = 0;
+   } else if (pm_iir  GEN6_PM_RP_DOWN_THRESHOLD) {
+   if (adj  0)
+   adj *= 2;
+   else
+   adj = -1;
+   new_delay = dev_priv-rps.cur_delay + adj;
+   } else { /* unknown event */
+   new_delay = dev_priv-rps.cur_delay;
+   }
 
/* sysfs frequency interfaces may have snuck in while servicing the
 * interrupt
 */
-   if (new_delay = dev_priv-rps.min_delay 
-   new_delay = dev_priv-rps.max_delay) {
-

[Intel-gfx] [PATCH 1/3] drm/i915: Fix __wait_seqno to use true infinite timeouts

2013-09-25 Thread Chris Wilson
When we switched to always using a timeout in conjunction with
wait_seqno, we lost the ability to detect missed interrupts. Since, we
have had issues with interrupts on a number of generations, and they are
required to be delivered in a timely fashion for a smooth UX, it is
important that we do log errors found in the wild and prevent the
display stalling for upwards of 1s every time the seqno interrupt is
missed.

Rather than continue to fix up the timeouts to work around the interface
impedence in wait_event_*(), open code the combination of
wait_event[_interruptible][_timeout], and use the exposed timer to
poll for seqno should we detect a lost interrupt.

v2: In order to satisfy the debug requirement of logging missed
interrupts with the real world requirments of making machines work even
if interrupts are hosed, we revert to polling after detecting a missed
interrupt.

v3: Throw in a debugfs interface to simulate broken hw not reporting
interrupts.

v4: s/EGAIN/EAGAIN/ (Imre)

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Reviewed-by: Imre Deak imre.d...@intel.com
---
 drivers/gpu/drm/i915/i915_debugfs.c   |  68 
 drivers/gpu/drm/i915/i915_drv.h   |   6 ++
 drivers/gpu/drm/i915/i915_gem.c   | 114 --
 drivers/gpu/drm/i915/i915_gpu_error.c |   1 +
 drivers/gpu/drm/i915/i915_irq.c   |  11 ++--
 5 files changed, 149 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index fcfa9884..0c339f0 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1897,6 +1897,72 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_ring_stop_fops,
i915_ring_stop_get, i915_ring_stop_set,
0x%08llx\n);
 
+static int
+i915_ring_missed_irq_get(void *data, u64 *val)
+{
+   struct drm_device *dev = data;
+   drm_i915_private_t *dev_priv = dev-dev_private;
+
+   *val = dev_priv-gpu_error.missed_irq_rings;
+   return 0;
+}
+
+static int
+i915_ring_missed_irq_set(void *data, u64 val)
+{
+   struct drm_device *dev = data;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   int ret;
+
+   /* Lock against concurrent debugfs callers */
+   ret = mutex_lock_interruptible(dev-struct_mutex);
+   if (ret)
+   return ret;
+   dev_priv-gpu_error.missed_irq_rings = val;
+   mutex_unlock(dev-struct_mutex);
+
+   return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(i915_ring_missed_irq_fops,
+   i915_ring_missed_irq_get, i915_ring_missed_irq_set,
+   0x%08llx\n);
+
+static int
+i915_ring_test_irq_get(void *data, u64 *val)
+{
+   struct drm_device *dev = data;
+   drm_i915_private_t *dev_priv = dev-dev_private;
+
+   *val = dev_priv-gpu_error.test_irq_rings;
+
+   return 0;
+}
+
+static int
+i915_ring_test_irq_set(void *data, u64 val)
+{
+   struct drm_device *dev = data;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   int ret;
+
+   DRM_DEBUG_DRIVER(Masking interrupts on rings 0x%08llx\n, val);
+
+   /* Lock against concurrent debugfs callers */
+   ret = mutex_lock_interruptible(dev-struct_mutex);
+   if (ret)
+   return ret;
+
+   dev_priv-gpu_error.test_irq_rings = val;
+   mutex_unlock(dev-struct_mutex);
+
+   return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(i915_ring_test_irq_fops,
+   i915_ring_test_irq_get, i915_ring_test_irq_set,
+   0x%08llx\n);
+
 #define DROP_UNBOUND 0x1
 #define DROP_BOUND 0x2
 #define DROP_RETIRE 0x4
@@ -2290,6 +2356,8 @@ static struct i915_debugfs_files {
{i915_min_freq, i915_min_freq_fops},
{i915_cache_sharing, i915_cache_sharing_fops},
{i915_ring_stop, i915_ring_stop_fops},
+   {i915_ring_missed_irq, i915_ring_missed_irq_fops},
+   {i915_ring_test_irq, i915_ring_test_irq_fops},
{i915_gem_drop_caches, i915_drop_caches_fops},
{i915_error_state, i915_error_state_fops},
{i915_next_seqno, i915_next_seqno_fops},
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e8ffd57..906f4fb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1022,6 +1022,9 @@ struct i915_gpu_error {
struct drm_i915_error_state *first_error;
struct work_struct work;
 
+
+   unsigned long missed_irq_rings;
+
/**
 * State variable and reset counter controlling the reset flow
 *
@@ -1060,6 +1063,9 @@ struct i915_gpu_error {
 
/* For gpu hang simulation. */
unsigned int stop_rings;
+
+   /* For missed irq/seqno simulation. */
+   unsigned int test_irq_rings;
 };
 
 enum modeset_restore {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fa3b373..40e1293 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ 

Re: [Intel-gfx] [PATCH] drm/i915: we can't call move_to_active before intel_ring_begin

2013-09-25 Thread Chris Wilson
On Wed, Sep 25, 2013 at 10:11:35AM -0700, Ben Widawsky wrote:
 On Wed, Sep 25, 2013 at 05:05:15PM +0200, Daniel Vetter wrote:
  Otherwise we can die in a fire of not-yet-allocated lazy requests when
  we expect them to be there:
 
 And Chris accuses me of violating Rusty's rules. This is an extremely
 ugly caveat to put in an interface given that active tracking and ring
 dispatch should have little connection.
 
 
 Isn't it much simpler to just call intel_ring_alloc_seqno during
 move_to_active?

Daniel keeps harping on about the potential allocation resulting in the
shrinker stealing vma and pages. All because we cheated and dropped the
pin early during reservation...
-Chris

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


Re: [Intel-gfx] [PATCH 5/6] drm/i915: Use the new vm [un]bind functions

2013-09-25 Thread Ben Widawsky
On Wed, Sep 25, 2013 at 01:42:21PM +0200, Daniel Vetter wrote:
 On Tue, Sep 24, 2013 at 09:58:00AM -0700, Ben Widawsky wrote:
  From: Ben Widawsky b...@bwidawsk.net
  
  Building on the last patch which created the new function pointers in
  the VM for bind/unbind, here we actually put those new function pointers
  to use.
  
  Split out as a separate patch to aid in review. I'm fine with squashing
  into the previous patch if people request it.
  
  v2: Updated to address the smart ggtt which can do aliasing as needed
  Make sure we bind to global gtt when mappable and fenceable. I thought
  we could get away without this initialy, but we cannot.
  
  v3: Make the global GTT binding explicitly use the ggtt VM for
  bind_vma(). While at it, use the new ggtt_vma helper (Chris)
  
  v4: Make the code support the secure dispatch flag, which requires
  special handling during execbuf. This was fixed (incorrectly) later in
  the series, but having it here earlier in the series should be perfectly
  acceptable. (Chris)
  Move do_switch over to the new, special ggtt_vma interface.
  
  v5: Don't use a local variable (or assertion) when setting the batch
  object to the global GTT during secure dispatch (Chris)
  
  v6: Caclulate the exec offset for the secure case (Bug fix missed on
  v4). (Chris)
  Remove redundant check for has_global_gtt_mapping, since it is done in
  bind_vma.
  
  v7: Remove now unused dev_priv in do_switch
  Don't pass the vm to ggtt_offset (error from v6 which I should have
  caught before sending).
  
  v8: Assert, and rework the SNB workaround (to make it a bit clearer)
  code to make sure the VM can't be anything but the GGTT.
  
  v9: Fixing more bugs which can't exist yet on the behest of Chris. Make
  sure that the batch object is properly bound, and added to the global
  VM's active list - for when we use non-global VMs. (Chris) Note that
  there is an ongoing confusion about what bugs existed, but the known
  bugs are fixed here.
  
  v10: Nitpicks on whitespacing etc. introduced in v9 (Chris)
  
  Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
  Signed-off-by: Ben Widawsky b...@bwidawsk.net
 
 
 Second is the death of the -insert_entries/-clear_range vfuncs. We
 _need_ them in the internal tree and you really can't just kill them. If
 you want to, you need to follow three steps:
 
 1. Create new interfaces in the public tree, use the on public platforms
 but keeep the old interfaces working.
 
 2. Convert the -internal platforms over.
 
 3. Remove old interface.
 
 Doing 3. before 2. is bonghits and will result in the internal tree being
 broken a bit in-between. Since I'm supposed to maintain that I'll undo the
 damage here to be able to do a rebase.
 
 Cheers, Daniel

As I've now stated multiple times over the course of the last 5 months -
I am fine with you not merging this. It's the right thing to do, but
it seems neither you or I have time to do it in the right way. Sometimes
reality gets in the way what's desirable.

 
  ---
   drivers/gpu/drm/i915/i915_drv.h|  9 -
   drivers/gpu/drm/i915/i915_gem.c| 33 +++-
   drivers/gpu/drm/i915/i915_gem_context.c|  6 ++-
   drivers/gpu/drm/i915/i915_gem_execbuffer.c | 61 
  --
   drivers/gpu/drm/i915/i915_gem_gtt.c| 48 ++-
   5 files changed, 62 insertions(+), 95 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/i915_drv.h 
  b/drivers/gpu/drm/i915/i915_drv.h
  index 9995cdb..e8ae8fd 100644
  --- a/drivers/gpu/drm/i915/i915_drv.h
  +++ b/drivers/gpu/drm/i915/i915_drv.h
  @@ -2102,17 +2102,8 @@ int i915_gem_context_destroy_ioctl(struct drm_device 
  *dev, void *data,
   
   /* i915_gem_gtt.c */
   void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev);
  -void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
  -   struct drm_i915_gem_object *obj,
  -   enum i915_cache_level cache_level);
  -void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt,
  - struct drm_i915_gem_object *obj);
  -
   void i915_gem_restore_gtt_mappings(struct drm_device *dev);
   int __must_check i915_gem_gtt_prepare_object(struct drm_i915_gem_object 
  *obj);
  -void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
  -   enum i915_cache_level cache_level);
  -void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj);
   void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj);
   void i915_gem_init_global_gtt(struct drm_device *dev);
   void i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long start,
  diff --git a/drivers/gpu/drm/i915/i915_gem.c 
  b/drivers/gpu/drm/i915/i915_gem.c
  index f6c8b0e..378d4ef 100644
  --- a/drivers/gpu/drm/i915/i915_gem.c
  +++ b/drivers/gpu/drm/i915/i915_gem.c
  @@ -2693,12 +2693,8 @@ int i915_vma_unbind(struct i915_vma *vma)
   
  trace_i915_vma_unbind(vma);
   
  -   if 

Re: [Intel-gfx] [PATCH] drm/i915/vlv: hack to init backlight regs if BIOS fails to do so

2013-09-25 Thread Jani Nikula
On Wed, 25 Sep 2013, Jesse Barnes jbar...@virtuousgeek.org wrote:
 Still digging up the actual VBT info for this, but wanted to get this
 out there for testing, or in case others are also bugged by this.

I had a look at this a few weeks back. The VBT value for max backlight
is in Hz (as is the value you get through opregion) and transforming
that into the value the registers eat needs some digging. I tried, but
none of the real world examples of VBT and PWM freq matched any of that,
so I moved on...

 This can happen if you boot with an external display connected.  In that
 case, the attached eDP backlight modulation frequency may not be
 programmed, so we need to use something (in this case the value my BIOS
 normally programs with just the internal display enabled).

Something similar is required for non-vlv ChromeOS stuff too AFAIK.

 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
 ---
  drivers/gpu/drm/i915/intel_panel.c | 17 +
  1 file changed, 17 insertions(+)

 diff --git a/drivers/gpu/drm/i915/intel_panel.c 
 b/drivers/gpu/drm/i915/intel_panel.c
 index 3bc89a6..a3536785 100644
 --- a/drivers/gpu/drm/i915/intel_panel.c
 +++ b/drivers/gpu/drm/i915/intel_panel.c
 @@ -372,6 +372,9 @@ static u32 i915_read_blc_pwm_ctl(struct drm_device *dev)
   I915_WRITE(BLC_PWM_CTL2,
  dev_priv-regfile.saveBLC_PWM_CTL2);
   }
 +
 + if (IS_VALLEYVIEW(dev)  !val)
 + val = 0x;

Huh, that's a lot... why don't you use the same value here and below?

In fact, it should be sufficient to do the hack right here, as this gets
called through intel_panel_setup_backlight(). Then again, this hole
function is a kludge... :/

   }
  
   return val;
 @@ -629,10 +632,24 @@ set_level:
   spin_unlock_irqrestore(dev_priv-backlight.lock, flags);
  }
  
 +/* FIXME: use VBT vals to init PWM_CTL and PWM_CTL2 correctly */
 +static void intel_panel_init_backlight_regs(struct drm_device *dev)
 +{
 + struct drm_i915_private *dev_priv = dev-dev_private;
 +
 + if (IS_VALLEYVIEW(dev)) {
 + u32 cur_val = I915_READ(BLC_PWM_CTL) 
 + ~BACKLIGHT_DUTY_CYCLE_MASK;

That should be without the NOT, right?

 + I915_WRITE(BLC_PWM_CTL, (0xf42  16) | cur_val);
 + }
 +}
 +
  static void intel_panel_init_backlight(struct drm_device *dev)
  {
   struct drm_i915_private *dev_priv = dev-dev_private;
  
 + intel_panel_init_backlight_regs(dev);
 +
   dev_priv-backlight.level = intel_panel_get_backlight(dev);
   dev_priv-backlight.enabled = dev_priv-backlight.level != 0;
  }
 -- 
 1.8.3.1

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


Re: [Intel-gfx] [PATCH] drm/i915/vlv: hack to init backlight regs if BIOS fails to do so

2013-09-25 Thread Jesse Barnes
On Wed, 25 Sep 2013 20:18:39 +0300
Jani Nikula jani.nik...@linux.intel.com wrote:

 On Wed, 25 Sep 2013, Jesse Barnes jbar...@virtuousgeek.org wrote:
  Still digging up the actual VBT info for this, but wanted to get this
  out there for testing, or in case others are also bugged by this.
 
 I had a look at this a few weeks back. The VBT value for max backlight
 is in Hz (as is the value you get through opregion) and transforming
 that into the value the registers eat needs some digging. I tried, but
 none of the real world examples of VBT and PWM freq matched any of that,
 so I moved on...
 
  This can happen if you boot with an external display connected.  In that
  case, the attached eDP backlight modulation frequency may not be
  programmed, so we need to use something (in this case the value my BIOS
  normally programs with just the internal display enabled).
 
 Something similar is required for non-vlv ChromeOS stuff too AFAIK.
 
  Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
  ---
   drivers/gpu/drm/i915/intel_panel.c | 17 +
   1 file changed, 17 insertions(+)
 
  diff --git a/drivers/gpu/drm/i915/intel_panel.c 
  b/drivers/gpu/drm/i915/intel_panel.c
  index 3bc89a6..a3536785 100644
  --- a/drivers/gpu/drm/i915/intel_panel.c
  +++ b/drivers/gpu/drm/i915/intel_panel.c
  @@ -372,6 +372,9 @@ static u32 i915_read_blc_pwm_ctl(struct drm_device *dev)
  I915_WRITE(BLC_PWM_CTL2,
 dev_priv-regfile.saveBLC_PWM_CTL2);
  }
  +
  +   if (IS_VALLEYVIEW(dev)  !val)
  +   val = 0x;
 
 Huh, that's a lot... why don't you use the same value here and below?
 
 In fact, it should be sufficient to do the hack right here, as this gets
 called through intel_panel_setup_backlight(). Then again, this hole
 function is a kludge... :/

Simply doing it here isn't enough, because we never actually set the
modulation freq bits.  But yes, we could use the same magic value in
both places.  Seems to work either way though.

 
  }
   
  return val;
  @@ -629,10 +632,24 @@ set_level:
  spin_unlock_irqrestore(dev_priv-backlight.lock, flags);
   }
   
  +/* FIXME: use VBT vals to init PWM_CTL and PWM_CTL2 correctly */
  +static void intel_panel_init_backlight_regs(struct drm_device *dev)
  +{
  +   struct drm_i915_private *dev_priv = dev-dev_private;
  +
  +   if (IS_VALLEYVIEW(dev)) {
  +   u32 cur_val = I915_READ(BLC_PWM_CTL) 
  +   ~BACKLIGHT_DUTY_CYCLE_MASK;
 
 That should be without the NOT, right?

No I'm explicitly ignoring the low 16 bits, trying to set the bits for
the modulation frequency.  Without those, setting a value just gives
you a blinking display as the backlight goes on and off about once per
500ms.


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


Re: [Intel-gfx] [PATCH] drm/i915/vlv: add VLV specific clock_get function v3

2013-09-25 Thread Jesse Barnes
On Mon, 23 Sep 2013 21:01:44 +0300
Ville Syrjälä ville.syrj...@linux.intel.com wrote:

 On Fri, Sep 20, 2013 at 11:29:32AM -0700, Jesse Barnes wrote:
  Calculation is a little different than other platforms.
  
  v2: update to use port_clock instead
  rebase on top of Ville's changes
  v3: update to new port_clock semantics - don't divide by
  pixel_multiplier (Ville)
  
  References: https://bugs.freedesktop.org/show_bug.cgi?id=67345
  Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
  ---
   drivers/gpu/drm/i915/intel_display.c |   33 
  -
   1 file changed, 32 insertions(+), 1 deletion(-)
  
  diff --git a/drivers/gpu/drm/i915/intel_display.c 
  b/drivers/gpu/drm/i915/intel_display.c
  index 7eecf37..e5c9c1c 100644
  --- a/drivers/gpu/drm/i915/intel_display.c
  +++ b/drivers/gpu/drm/i915/intel_display.c
  @@ -5048,6 +5048,34 @@ static void i9xx_get_pfit_config(struct intel_crtc 
  *crtc,
  I915_READ(LVDS)  LVDS_BORDER_ENABLE;
   }
   
  +static void vlv_crtc_clock_get(struct intel_crtc *crtc,
  +  struct intel_crtc_config *pipe_config)
  +{
  +   struct drm_device *dev = crtc-base.dev;
  +   struct drm_i915_private *dev_priv = dev-dev_private;
  +   int pipe = pipe_config-cpu_transcoder;
  +   intel_clock_t clock;
  +   u32 mdiv;
  +   int refclk = 10, fastclk, update_rate;
  +
  +   mutex_lock(dev_priv-dpio_lock);
  +   mdiv = vlv_dpio_read(dev_priv, pipe, DPIO_DIV(pipe));
  +   mutex_unlock(dev_priv-dpio_lock);
  +
  +   clock.m1 = (mdiv  DPIO_M1DIV_SHIFT)  7;
  +   clock.m2 = mdiv  DPIO_M2DIV_MASK;
  +   clock.n = (mdiv  DPIO_N_SHIFT)  0xf;
  +   clock.p1 = (mdiv  DPIO_P1_SHIFT)  7;
  +   clock.p2 = (mdiv  DPIO_P2_SHIFT)  0x1f;
  +
  +   update_rate = refclk / clock.n;
  +   clock.vco = update_rate * clock.m1 * clock.m2;
  +   fastclk = clock.vco / clock.p1 / clock.p2;
  +   clock.dot = (2 * fastclk);
  +
  +   pipe_config-port_clock = clock.dot / 10;
 
 Looks like it should get roughly the right answer, but I don't see much
 point in all the intermediate results.
 
 If you want to keep some of them for clarity, then I think this should
 be enough:
 
 clock.vco = refclk * clock.m / clock.n;
 clock.dot = clock.vco / clock.p; /* fast clock */
 pipe_config-port_clock = clock.dot / 5;
 
 Although calling the fast clock dot is a bit wrong, but I think it's
 fine here, especially as it matches what I have in mind for
 vlv_find_best_dpll(). I had to rewrite that sucker a bit to make it
 compatible with my brain ;) I'll send a patch for that ASAP.

The other advantage of having it all spelled out like this is that it
matches the freq calculation spreadsheet I sourced it from.

So I'd prefer to keep this as is, if I can have your r-b anyway. :)

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] [PATCH v3 3/4] ACPI / video: Do not register backlight if win8 and native interface exists

2013-09-25 Thread Rafael J. Wysocki
On Tuesday, September 24, 2013 05:47:31 PM Aaron Lu wrote:
 According to Matthew Garrett, Windows 8 leaves backlight control up
 to individual graphics drivers rather than making ACPI calls itself.
 There's plenty of evidence to suggest that the Intel driver for
 Windows [8] doesn't use the ACPI interface, including the fact that
 it's broken on a bunch of machines when the OS claims to support
 Windows 8.  The simplest thing to do appears to be to disable the
 ACPI backlight interface on these systems.
 
 So for Win8 systems, if there is native backlight control interface
 registered by GPU driver, ACPI video will not register its own. For
 users who prefer to keep ACPI video's backlight interface, the existing
 kernel cmdline option acpi_backlight=video can be used.

I think the idea is to use the aggressive default for now and we can switch the
default back to the current behavior before the merge window in case there are
too many problems with it?

Rafael


 Signed-off-by: Aaron Lu aaron...@intel.com
 Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com
 Tested-by: Yves-Alexis Perez cor...@debian.org
 ---
  drivers/acpi/internal.h |  5 ++---
  drivers/acpi/video.c| 10 +-
  drivers/acpi/video_detect.c | 14 --
  3 files changed, 19 insertions(+), 10 deletions(-)
 
 diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
 index 20f4233..453ae8d 100644
 --- a/drivers/acpi/internal.h
 +++ b/drivers/acpi/internal.h
 @@ -169,9 +169,8 @@ int acpi_create_platform_device(struct acpi_device *adev,
   Video
-- 
 */
  #if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
 -bool acpi_video_backlight_quirks(void);
 -#else
 -static inline bool acpi_video_backlight_quirks(void) { return false; }
 +bool acpi_osi_is_win8(void);
 +bool acpi_video_verify_backlight_support(void);
  #endif
  
  #endif /* _ACPI_INTERNAL_H_ */
 diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
 index 3bd1eaa..343db59 100644
 --- a/drivers/acpi/video.c
 +++ b/drivers/acpi/video.c
 @@ -1256,8 +1256,8 @@ acpi_video_switch_brightness(struct acpi_video_device 
 *device, int event)
   unsigned long long level_current, level_next;
   int result = -EINVAL;
  
 - /* no warning message if acpi_backlight=vendor is used */
 - if (!acpi_video_backlight_support())
 + /* no warning message if acpi_backlight=vendor or a quirk is used */
 + if (!acpi_video_verify_backlight_support())
   return 0;
  
   if (!device-brightness)
 @@ -1386,13 +1386,13 @@ acpi_video_bus_get_devices(struct acpi_video_bus 
 *video,
  static int acpi_video_bus_start_devices(struct acpi_video_bus *video)
  {
   return acpi_video_bus_DOS(video, 0,
 -   acpi_video_backlight_quirks() ? 1 : 0);
 +   acpi_osi_is_win8() ? 1 : 0);
  }
  
  static int acpi_video_bus_stop_devices(struct acpi_video_bus *video)
  {
   return acpi_video_bus_DOS(video, 0,
 -   acpi_video_backlight_quirks() ? 0 : 1);
 +   acpi_osi_is_win8() ? 0 : 1);
  }
  
  static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
 @@ -1558,7 +1558,7 @@ acpi_video_bus_match(acpi_handle handle, u32 level, 
 void *context,
  
  static void acpi_video_dev_register_backlight(struct acpi_video_device 
 *device)
  {
 - if (acpi_video_backlight_support()) {
 + if (acpi_video_verify_backlight_support()) {
   struct backlight_properties props;
   struct pci_dev *pdev;
   acpi_handle acpi_parent;
 diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
 index 940edbf..23d7d26 100644
 --- a/drivers/acpi/video_detect.c
 +++ b/drivers/acpi/video_detect.c
 @@ -37,6 +37,7 @@
  #include linux/acpi.h
  #include linux/dmi.h
  #include linux/pci.h
 +#include linux/backlight.h
  
  #include internal.h
  
 @@ -233,11 +234,11 @@ static void acpi_video_caps_check(void)
   acpi_video_get_capabilities(NULL);
  }
  
 -bool acpi_video_backlight_quirks(void)
 +bool acpi_osi_is_win8(void)
  {
   return acpi_gbl_osi_data = ACPI_OSI_WIN_8;
  }
 -EXPORT_SYMBOL(acpi_video_backlight_quirks);
 +EXPORT_SYMBOL(acpi_osi_is_win8);
  
  /* Promote the vendor interface instead of the generic video module.
   * This function allow DMI blacklists to be implemented by externals
 @@ -283,6 +284,15 @@ int acpi_video_backlight_support(void)
  }
  EXPORT_SYMBOL(acpi_video_backlight_support);
  
 +bool acpi_video_verify_backlight_support(void)
 +{
 + if (!(acpi_video_support  ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO) 
 + acpi_osi_is_win8()  backlight_device_registered(BACKLIGHT_RAW))
 + return false;
 + return acpi_video_backlight_support();
 +}
 +EXPORT_SYMBOL(acpi_video_verify_backlight_support);
 +
  /*
   * Use acpi_backlight=vendor/video to 

Re: [Intel-gfx] [PATCH] drm/i915/vlv: hack to init backlight regs if BIOS fails to do so

2013-09-25 Thread Jesse Barnes
On Wed, 25 Sep 2013 20:18:39 +0300
Jani Nikula jani.nik...@linux.intel.com wrote:

 On Wed, 25 Sep 2013, Jesse Barnes jbar...@virtuousgeek.org wrote:
  Still digging up the actual VBT info for this, but wanted to get this
  out there for testing, or in case others are also bugged by this.
 
 I had a look at this a few weeks back. The VBT value for max backlight
 is in Hz (as is the value you get through opregion) and transforming
 that into the value the registers eat needs some digging. I tried, but
 none of the real world examples of VBT and PWM freq matched any of that,
 so I moved on...
 
  This can happen if you boot with an external display connected.  In that
  case, the attached eDP backlight modulation frequency may not be
  programmed, so we need to use something (in this case the value my BIOS
  normally programs with just the internal display enabled).
 
 Something similar is required for non-vlv ChromeOS stuff too AFAIK.
 
  Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
  ---
   drivers/gpu/drm/i915/intel_panel.c | 17 +
   1 file changed, 17 insertions(+)
 
  diff --git a/drivers/gpu/drm/i915/intel_panel.c 
  b/drivers/gpu/drm/i915/intel_panel.c
  index 3bc89a6..a3536785 100644
  --- a/drivers/gpu/drm/i915/intel_panel.c
  +++ b/drivers/gpu/drm/i915/intel_panel.c
  @@ -372,6 +372,9 @@ static u32 i915_read_blc_pwm_ctl(struct drm_device *dev)
  I915_WRITE(BLC_PWM_CTL2,
 dev_priv-regfile.saveBLC_PWM_CTL2);
  }
  +
  +   if (IS_VALLEYVIEW(dev)  !val)
  +   val = 0x;
 
 Huh, that's a lot... why don't you use the same value here and below?
 
 In fact, it should be sufficient to do the hack right here, as this gets
 called through intel_panel_setup_backlight(). Then again, this hole
 function is a kludge... :/
 
  }
   
  return val;
  @@ -629,10 +632,24 @@ set_level:
  spin_unlock_irqrestore(dev_priv-backlight.lock, flags);
   }
   
  +/* FIXME: use VBT vals to init PWM_CTL and PWM_CTL2 correctly */
  +static void intel_panel_init_backlight_regs(struct drm_device *dev)
  +{
  +   struct drm_i915_private *dev_priv = dev-dev_private;
  +
  +   if (IS_VALLEYVIEW(dev)) {
  +   u32 cur_val = I915_READ(BLC_PWM_CTL) 
  +   ~BACKLIGHT_DUTY_CYCLE_MASK;
 
 That should be without the NOT, right?

Oops yes, rather than preserving the value I'm about to clobber... :)

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


Re: [Intel-gfx] [PATCH] drm/i915/vlv: hack to init backlight regs if BIOS fails to do so

2013-09-25 Thread Daniel Vetter
On Wed, Sep 25, 2013 at 08:18:39PM +0300, Jani Nikula wrote:
 On Wed, 25 Sep 2013, Jesse Barnes jbar...@virtuousgeek.org wrote:
  Still digging up the actual VBT info for this, but wanted to get this
  out there for testing, or in case others are also bugged by this.
 
 I had a look at this a few weeks back. The VBT value for max backlight
 is in Hz (as is the value you get through opregion) and transforming
 that into the value the registers eat needs some digging. I tried, but
 none of the real world examples of VBT and PWM freq matched any of that,
 so I moved on...
 
  This can happen if you boot with an external display connected.  In that
  case, the attached eDP backlight modulation frequency may not be
  programmed, so we need to use something (in this case the value my BIOS
  normally programs with just the internal display enabled).
 
 Something similar is required for non-vlv ChromeOS stuff too AFAIK.

Afaik ChromeOS doesn't have a vbt, so I think we need to shovel some
failsafe (yeah, failsafe and backlight doesn't compute, I know) default
into the regs in case all else fails.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Fix pre-CTG vblank counter

2013-09-25 Thread Daniel Vetter
On Wed, Sep 25, 2013 at 07:55:26PM +0300, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä ville.syrj...@linux.intel.com
 
 The old style frame counter increments at the start of active video.
 However for i915_get_vblank_counter() we want a counter that increments
 at the start of vblank.
 
 Fortunately the low frame counter register also contains the pixel
 counter for the current frame. We can can compare that against the
 vblank start pixel count to determine if we need to increment the
 frame counter by 1 to get the correct answer.
 
 Also reorganize the function pointer assignments in intel_irq_init() a
 bit to avoid confusing people.
 
 Cc: Mario Kleiner mario.klei...@tuebingen.mpg.de
 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
 
 Just another small vblank related gem I forgot to polish up and send
 out until Imre started asking me questions about the vblank counter
 functions.

Hm, I've thought the magic fixup code does take care of that for us? But I
agree that we should do this explicitly ...

This could explain some of the strange vblank timestamp off failures QA
has reported (if there's too much delay and the fixup doesn't fire any
more), care to attach this patch to the relevant bug reports? Searching
for ts jitter + pre-gen5 should be good enough.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Mask LPSP to get PSR working even with Power Well in use by audio.

2013-09-25 Thread Rodrigo Vivi
Power Well in use forces constantly PSR to exit.
On recent Kernel I noticed that PSR Performance Counter was always 0
indicating that PSR was never really achieved.
By masking LPSP, PSR can work normally and save power on Haswell.

Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
 drivers/gpu/drm/i915/intel_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 79c14e2..2c555f9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1467,7 +1467,7 @@ static void intel_edp_psr_setup(struct intel_dp *intel_dp)
 
/* Avoid continuous PSR exit by masking memup and hpd */
I915_WRITE(EDP_PSR_DEBUG_CTL, EDP_PSR_DEBUG_MASK_MEMUP |
-  EDP_PSR_DEBUG_MASK_HPD);
+  EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP);
 
intel_dp-psr_setup_done = true;
 }
-- 
1.7.11.7

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


Re: [Intel-gfx] [PATCH] drm/i915: Mask LPSP to get PSR working even with Power Well in use by audio.

2013-09-25 Thread Rodrigo Vivi
Hi Daniel,

Please consider to accept this for -fixes, otherwise PSR will never
work on Haswell on 3.12.

Thanks,
Rodrigo.

On Wed, Sep 25, 2013 at 5:51 PM, Rodrigo Vivi rodrigo.v...@gmail.com wrote:
 Power Well in use forces constantly PSR to exit.
 On recent Kernel I noticed that PSR Performance Counter was always 0
 indicating that PSR was never really achieved.
 By masking LPSP, PSR can work normally and save power on Haswell.

 Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
 ---
  drivers/gpu/drm/i915/intel_dp.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index 79c14e2..2c555f9 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -1467,7 +1467,7 @@ static void intel_edp_psr_setup(struct intel_dp 
 *intel_dp)

 /* Avoid continuous PSR exit by masking memup and hpd */
 I915_WRITE(EDP_PSR_DEBUG_CTL, EDP_PSR_DEBUG_MASK_MEMUP |
 -  EDP_PSR_DEBUG_MASK_HPD);
 +  EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP);

 intel_dp-psr_setup_done = true;
  }
 --
 1.7.11.7




-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915/vlv: add VLV specific clock_get function v3

2013-09-25 Thread Daniel Vetter
On Wed, Sep 25, 2013 at 10:38:33AM -0700, Jesse Barnes wrote:
 On Mon, 23 Sep 2013 21:01:44 +0300
 Ville Syrjälä ville.syrj...@linux.intel.com wrote:
 
  On Fri, Sep 20, 2013 at 11:29:32AM -0700, Jesse Barnes wrote:
   Calculation is a little different than other platforms.
   
   v2: update to use port_clock instead
   rebase on top of Ville's changes
   v3: update to new port_clock semantics - don't divide by
   pixel_multiplier (Ville)
   
   References: https://bugs.freedesktop.org/show_bug.cgi?id=67345
   Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
   ---
drivers/gpu/drm/i915/intel_display.c |   33 
   -
1 file changed, 32 insertions(+), 1 deletion(-)
   
   diff --git a/drivers/gpu/drm/i915/intel_display.c 
   b/drivers/gpu/drm/i915/intel_display.c
   index 7eecf37..e5c9c1c 100644
   --- a/drivers/gpu/drm/i915/intel_display.c
   +++ b/drivers/gpu/drm/i915/intel_display.c
   @@ -5048,6 +5048,34 @@ static void i9xx_get_pfit_config(struct intel_crtc 
   *crtc,
 I915_READ(LVDS)  LVDS_BORDER_ENABLE;
}

   +static void vlv_crtc_clock_get(struct intel_crtc *crtc,
   +struct intel_crtc_config *pipe_config)
   +{
   + struct drm_device *dev = crtc-base.dev;
   + struct drm_i915_private *dev_priv = dev-dev_private;
   + int pipe = pipe_config-cpu_transcoder;
   + intel_clock_t clock;
   + u32 mdiv;
   + int refclk = 10, fastclk, update_rate;
   +
   + mutex_lock(dev_priv-dpio_lock);
   + mdiv = vlv_dpio_read(dev_priv, pipe, DPIO_DIV(pipe));
   + mutex_unlock(dev_priv-dpio_lock);
   +
   + clock.m1 = (mdiv  DPIO_M1DIV_SHIFT)  7;
   + clock.m2 = mdiv  DPIO_M2DIV_MASK;
   + clock.n = (mdiv  DPIO_N_SHIFT)  0xf;
   + clock.p1 = (mdiv  DPIO_P1_SHIFT)  7;
   + clock.p2 = (mdiv  DPIO_P2_SHIFT)  0x1f;
   +
   + update_rate = refclk / clock.n;
   + clock.vco = update_rate * clock.m1 * clock.m2;
   + fastclk = clock.vco / clock.p1 / clock.p2;
   + clock.dot = (2 * fastclk);
   +
   + pipe_config-port_clock = clock.dot / 10;
  
  Looks like it should get roughly the right answer, but I don't see much
  point in all the intermediate results.
  
  If you want to keep some of them for clarity, then I think this should
  be enough:
  
  clock.vco = refclk * clock.m / clock.n;
  clock.dot = clock.vco / clock.p; /* fast clock */
  pipe_config-port_clock = clock.dot / 5;
  
  Although calling the fast clock dot is a bit wrong, but I think it's
  fine here, especially as it matches what I have in mind for
  vlv_find_best_dpll(). I had to rewrite that sucker a bit to make it
  compatible with my brain ;) I'll send a patch for that ASAP.
 
 The other advantage of having it all spelled out like this is that it
 matches the freq calculation spreadsheet I sourced it from.
 
 So I'd prefer to keep this as is, if I can have your r-b anyway. :)

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


Re: [Intel-gfx] [PATCH] drm/i915: Mask LPSP to get PSR working even with Power Well in use by audio.

2013-09-25 Thread Daniel Vetter
On Wed, Sep 25, 2013 at 10:59 PM, Rodrigo Vivi rodrigo.v...@gmail.com wrote:
 Please consider to accept this for -fixes, otherwise PSR will never
 work on Haswell on 3.12.

You know the drill: A feature regressed and no one noticed, which
means we are lacking a fully automated testcase. I guess we need to
expose in debugfs somewhere if the edp panel can do psr (if we don't
do that already) to be able to skip the test correctly and then check
with a little igt testcase that we actually achieve psr residency.
Also please poke QA to make sure they actually have a hsw platform
with psr panel.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915/vlv: fix up broken precision in vlv_crtc_clock_get

2013-09-25 Thread Daniel Vetter
On Wed, Sep 25, 2013 at 02:24:01PM -0700, Jesse Barnes wrote:
 From: Chris Wilson ch...@chris-wilson.co.uk
 
 With some divider values we end up with the wrong result.  So remove the
 intermediates (like Ville suggested in the first place) to get the right
 answer.
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org

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


Re: [Intel-gfx] [PATCH] drm/i915/vlv: add VLV specific clock_get function v3

2013-09-25 Thread Chris Wilson
On Wed, Sep 25, 2013 at 11:00:20PM +0200, Daniel Vetter wrote:
 On Wed, Sep 25, 2013 at 10:38:33AM -0700, Jesse Barnes wrote:
  On Mon, 23 Sep 2013 21:01:44 +0300
  Ville Syrjälä ville.syrj...@linux.intel.com wrote:
  
   On Fri, Sep 20, 2013 at 11:29:32AM -0700, Jesse Barnes wrote:
Calculation is a little different than other platforms.

v2: update to use port_clock instead
rebase on top of Ville's changes
v3: update to new port_clock semantics - don't divide by
pixel_multiplier (Ville)

References: https://bugs.freedesktop.org/show_bug.cgi?id=67345
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
 drivers/gpu/drm/i915/intel_display.c |   33 
-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 7eecf37..e5c9c1c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5048,6 +5048,34 @@ static void i9xx_get_pfit_config(struct 
intel_crtc *crtc,
I915_READ(LVDS)  LVDS_BORDER_ENABLE;
 }
 
+static void vlv_crtc_clock_get(struct intel_crtc *crtc,
+  struct intel_crtc_config *pipe_config)
+{
+   struct drm_device *dev = crtc-base.dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   int pipe = pipe_config-cpu_transcoder;
+   intel_clock_t clock;
+   u32 mdiv;
+   int refclk = 10, fastclk, update_rate;
+
+   mutex_lock(dev_priv-dpio_lock);
+   mdiv = vlv_dpio_read(dev_priv, pipe, DPIO_DIV(pipe));
+   mutex_unlock(dev_priv-dpio_lock);
+
+   clock.m1 = (mdiv  DPIO_M1DIV_SHIFT)  7;
+   clock.m2 = mdiv  DPIO_M2DIV_MASK;
+   clock.n = (mdiv  DPIO_N_SHIFT)  0xf;
+   clock.p1 = (mdiv  DPIO_P1_SHIFT)  7;
+   clock.p2 = (mdiv  DPIO_P2_SHIFT)  0x1f;
+
+   update_rate = refclk / clock.n;
+   clock.vco = update_rate * clock.m1 * clock.m2;
+   fastclk = clock.vco / clock.p1 / clock.p2;
+   clock.dot = (2 * fastclk);
+
+   pipe_config-port_clock = clock.dot / 10;
   
   Looks like it should get roughly the right answer, but I don't see much
   point in all the intermediate results.
   
   If you want to keep some of them for clarity, then I think this should
   be enough:
   
   clock.vco = refclk * clock.m / clock.n;
   clock.dot = clock.vco / clock.p; /* fast clock */
   pipe_config-port_clock = clock.dot / 5;
   
   Although calling the fast clock dot is a bit wrong, but I think it's
   fine here, especially as it matches what I have in mind for
   vlv_find_best_dpll(). I had to rewrite that sucker a bit to make it
   compatible with my brain ;) I'll send a patch for that ASAP.
  
  The other advantage of having it all spelled out like this is that it
  matches the freq calculation spreadsheet I sourced it from.
  
  So I'd prefer to keep this as is, if I can have your r-b anyway. :)
 
 Queued for -next with Ville's irc-r-b, thanks for the patch.
 -Daniel

Did Jesse post a delta update to this, as #67345 still warns with v3.

For the record, this works - can't see why it would make a difference at
the moment though (probably some type is too small or loss of precision):

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 5741d48..5219fdc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5083,10 +5083,10 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc,
 {
struct drm_device *dev = crtc-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
+   const int refclk = 10;
int pipe = pipe_config-cpu_transcoder;
intel_clock_t clock;
u32 mdiv;
-   int refclk = 10, fastclk, update_rate;
 
mutex_lock(dev_priv-dpio_lock);
mdiv = vlv_dpio_read(dev_priv, pipe, DPIO_DIV(pipe));
@@ -5098,12 +5098,16 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc,
clock.p1 = (mdiv  DPIO_P1_SHIFT)  7;
clock.p2 = (mdiv  DPIO_P2_SHIFT)  0x1f;
 
-   update_rate = refclk / clock.n;
-   clock.vco = update_rate * clock.m1 * clock.m2;
-   fastclk = clock.vco / clock.p1 / clock.p2;
-   clock.dot = (2 * fastclk);
+   clock.vco = refclk * clock.m1 * clock.m2 / clock.n;
+   clock.dot = 2 * clock.vco / (clock.p1 * clock.p2);
 
pipe_config-port_clock = clock.dot / 10;
 }

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


Re: [Intel-gfx] [PATCH] drm/i915: Fix up usage of SHRINK_STOP

2013-09-25 Thread Dave Chinner
On Wed, Sep 25, 2013 at 02:00:02PM +0200, Daniel Vetter wrote:
 In
 
 commit 81e49f811404f428a9d9a63295a0c267e802fa12
 Author: Glauber Costa glom...@openvz.org
 Date:   Wed Aug 28 10:18:13 2013 +1000
 
 i915: bail out earlier when shrinker cannot acquire mutex
 
 SHRINK_STOP was added to tell the core shrinker code to bail out and
 go to the next shrinker since the i915 shrinker couldn't acquire
 required locks. But the SHRINK_STOP return code was added to the
 -count_objects callback and not the -scan_objects callback as it
 should have been, resulting in tons of dmesg noise like
 
 shrink_slab: i915_gem_inactive_scan+0x0/0x9c negative objects to delete 
 nr=-x
 
 Fix discusssed with Dave Chinner.

Acked-by: Dave Chinner dchin...@redhat.com

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 1/2] drm/i915: Print ring min freq scaling

2013-09-25 Thread Ben Widawsky
This allows us to keep track of the values being set if we want to tweak
this code.

Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/intel_pm.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d27eda6..31cf188 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3671,6 +3671,9 @@ void gen6_update_ring_freq(struct drm_device *dev)
ring_freq = (gpu_freq * 5 + 3) / 4;
ring_freq = max(min_ring_freq, ring_freq);
/* leave ia_freq as the default, chosen by cpufreq */
+
+   DRM_DEBUG_DRIVER(Setup min ring frequency %dMHz for GT 
freq %dMHz\n,
+ring_freq * 100, gpu_freq * 50);
} else {
/* On older processors, there is no separate ring
 * clock domain, so in order to boost the bandwidth
@@ -3684,6 +3687,9 @@ void gen6_update_ring_freq(struct drm_device *dev)
else
ia_freq = max_ia_freq - ((diff * 
scaling_factor) / 2);
ia_freq = DIV_ROUND_CLOSEST(ia_freq, 100);
+
+   DRM_DEBUG_DRIVER(Setup min ring frequency %dMHz for GT 
freq %dMHz\n,
+ia_freq * 100, gpu_freq * 50);
}
 
sandybridge_pcode_write(dev_priv,
-- 
1.8.4

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


[Intel-gfx] [PATCH 2/2] drm/i915: Use the real cpu max frequency for ring scaling

2013-09-25 Thread Ben Widawsky
The policy's max frequency is not equal to the CPU's max frequency. The
ring frequency is derived from the CPU frequency, and not the policy
frequency.

One example of how this may differ through sysfs. If the sysfs max
frequency is modified, that will be used for the max ring frequency
calculation.
(/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq). As far as I
know, no current governor uses anything but max as the default, but in
theory, they could. Similarly distributions might set policy as part of
their init process.

It's ideal to use the real frequency because when we're currently scaled
up on the GPU. In this case we likely want to race to idle, and using a
less than max ring frequency is non-optimal for this situation.

AFAIK, this patch should have no impact on a majority of people.

This behavior hasn't been changed since it was first introduced:
commit 23b2f8bb92feb83127679c53633def32d3108e70
Author: Jesse Barnes jbar...@virtuousgeek.org
Date:   Tue Jun 28 13:04:16 2011 -0700

drm/i915: load a ring frequency scaling table v3

CC: Jesse Barnes jbar...@virtuousgeek.org
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/intel_pm.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 31cf188..3212d3b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3639,16 +3639,21 @@ void gen6_update_ring_freq(struct drm_device *dev)
unsigned int gpu_freq;
unsigned int max_ia_freq, min_ring_freq;
int scaling_factor = 180;
+   struct cpufreq_policy *policy;
 
WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock));
 
-   max_ia_freq = cpufreq_quick_get_max(0);
-   /*
-* Default to measured freq if none found, PCU will ensure we don't go
-* over
-*/
-   if (!max_ia_freq)
+   policy = cpufreq_cpu_get(0);
+   if (policy) {
+   max_ia_freq = policy-cpuinfo.max_freq;
+   cpufreq_cpu_put(policy);
+   } else {
+   /*
+* Default to measured freq if none found, PCU will ensure we
+* don't go over
+*/
max_ia_freq = tsc_khz;
+   }
 
/* Convert from kHz to MHz */
max_ia_freq /= 1000;
-- 
1.8.4

___
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: Print ring min freq scaling

2013-09-25 Thread Chris Wilson
On Wed, Sep 25, 2013 at 04:35:32PM -0700, Ben Widawsky wrote:
 This allows us to keep track of the values being set if we want to tweak
 this code.
 
 Signed-off-by: Ben Widawsky b...@bwidawsk.net
 ---
  drivers/gpu/drm/i915/intel_pm.c | 6 ++
  1 file changed, 6 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index d27eda6..31cf188 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -3671,6 +3671,9 @@ void gen6_update_ring_freq(struct drm_device *dev)
   ring_freq = (gpu_freq * 5 + 3) / 4;
   ring_freq = max(min_ring_freq, ring_freq);
   /* leave ia_freq as the default, chosen by cpufreq */
 +
 + DRM_DEBUG_DRIVER(Setup min ring frequency %dMHz for GT 
 freq %dMHz\n,
 +  ring_freq * 100, gpu_freq * 50);
   } else {
   /* On older processors, there is no separate ring
* clock domain, so in order to boost the bandwidth
 @@ -3684,6 +3687,9 @@ void gen6_update_ring_freq(struct drm_device *dev)
   else
   ia_freq = max_ia_freq - ((diff * 
 scaling_factor) / 2);
   ia_freq = DIV_ROUND_CLOSEST(ia_freq, 100);
 +
 + DRM_DEBUG_DRIVER(Setup min ring frequency %dMHz for GT 
 freq %dMHz\n,
 +  ia_freq * 100, gpu_freq * 50);

This is not a ring freq, but a cpu core freq.
Also would be wise to point out that this information is in
/sys/kernel/debug/dri/0/i915_ring_freq_table
-Chris

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


Re: [Intel-gfx] [PATCH 2/2] drm/i915: Use the real cpu max frequency for ring scaling

2013-09-25 Thread Chris Wilson
On Wed, Sep 25, 2013 at 04:35:33PM -0700, Ben Widawsky wrote:
 The policy's max frequency is not equal to the CPU's max frequency. The
 ring frequency is derived from the CPU frequency, and not the policy
 frequency.
 
 One example of how this may differ through sysfs. If the sysfs max
 frequency is modified, that will be used for the max ring frequency
 calculation.
 (/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq). As far as I
 know, no current governor uses anything but max as the default, but in
 theory, they could. Similarly distributions might set policy as part of
 their init process.

Actually this seems to differ based on policy as well. The pstate may
disable turbo mode, in which case policy-max = policy-cpuinfo.max.
But excluding user intervention, it seems that max is what we want, and
it is possibly arguable that we do not want to push the core to turbo
for a GPU bound workload - though I'm not sure if the ring frequency
continues to scale with turbo cpu frequencies.
 
 It's ideal to use the real frequency because when we're currently scaled
 up on the GPU. In this case we likely want to race to idle, and using a
 less than max ring frequency is non-optimal for this situation.

Also note that scaling the cpu frequency cuts into our thermal headroom,
so for ULT devices it may not be clearly beneficial.
 
 AFAIK, this patch should have no impact on a majority of people.

And also obsolete since HSW.
 
 This behavior hasn't been changed since it was first introduced:
 commit 23b2f8bb92feb83127679c53633def32d3108e70
 Author: Jesse Barnes jbar...@virtuousgeek.org
 Date:   Tue Jun 28 13:04:16 2011 -0700
 
 drm/i915: load a ring frequency scaling table v3
 
 CC: Jesse Barnes jbar...@virtuousgeek.org
 Signed-off-by: Ben Widawsky b...@bwidawsk.net

FWIW,

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

I think using the real range of the CPU frequency rather than the
adjusted policy maximum is sensible. Just the hw implementation is just
a bit silly and it is not clear exactly what the best policy should be.
-Chris

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


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Print ring min freq scaling

2013-09-25 Thread Ben Widawsky
On Thu, Sep 26, 2013 at 12:49:37AM +0100, Chris Wilson wrote:
 On Wed, Sep 25, 2013 at 04:35:32PM -0700, Ben Widawsky wrote:
  This allows us to keep track of the values being set if we want to tweak
  this code.
  
  Signed-off-by: Ben Widawsky b...@bwidawsk.net
  ---
   drivers/gpu/drm/i915/intel_pm.c | 6 ++
   1 file changed, 6 insertions(+)
  
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index d27eda6..31cf188 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -3671,6 +3671,9 @@ void gen6_update_ring_freq(struct drm_device *dev)
  ring_freq = (gpu_freq * 5 + 3) / 4;
  ring_freq = max(min_ring_freq, ring_freq);
  /* leave ia_freq as the default, chosen by cpufreq */
  +
  +   DRM_DEBUG_DRIVER(Setup min ring frequency %dMHz for GT 
  freq %dMHz\n,
  +ring_freq * 100, gpu_freq * 50);
  } else {
  /* On older processors, there is no separate ring
   * clock domain, so in order to boost the bandwidth
  @@ -3684,6 +3687,9 @@ void gen6_update_ring_freq(struct drm_device *dev)
  else
  ia_freq = max_ia_freq - ((diff * 
  scaling_factor) / 2);
  ia_freq = DIV_ROUND_CLOSEST(ia_freq, 100);
  +
  +   DRM_DEBUG_DRIVER(Setup min ring frequency %dMHz for GT 
  freq %dMHz\n,
  +ia_freq * 100, gpu_freq * 50);
 
 This is not a ring freq, but a cpu core freq.
 Also would be wise to point out that this information is in
 /sys/kernel/debug/dri/0/i915_ring_freq_table
 -Chris
 
 -- 
 Chris Wilson, Intel Open Source Technology Centre

Ooops. I forgot about that interface. Ignore this patch.

You are incorrect in saying it's _just_ the CPU core frequency. This is
the ring and IA frequency on SNB + IVB. The frequency is tied to the CPU
core on SNB + IVB. It is not tied to the cpu core on Haswell.

To make the print more generic, and correct across the board it makes
sense to say we're setting the ring frequency (to me). Even the
programming guides typically call this, IA/ring frequency.

Above is only for posterity. Again, forget the patch.

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


Re: [Intel-gfx] [PATCH v3 4/4] thinkpad-acpi: fix handle locate for video and query of _BCL

2013-09-25 Thread Aaron Lu
On Wed, Sep 25, 2013 at 04:58:39PM -0300, Henrique de Moraes Holschuh wrote:
 On Tue, 24 Sep 2013, Aaron Lu wrote:
  locate handle for ACPI video by HID, the problem is, ACPI video node
  doesn't really have HID defined(i.e. no _HID control method is defined
 
 ACPI video is supposed to attach a virtual HID node (ACPI_VIDEO_HID) to ACPI
 video devices so as to keep the non-trivial video device detection logic in
 just one place instead of reinventing the wheel in every driver (which is
 always a recipe for disaster).
 
 When did that break?

I checked the git log for the commit to use tpacpi_acpi_handle_locate to
locate video controller's ACPI handle, it's:

commit 122f26726b5e16174bf8a707df14be1d93c49d62
Author: Henrique de Moraes Holschuh h...@hmh.eng.br
Date:   Mon Aug 9 23:48:18 2010 -0300

thinkpad-acpi: find ACPI video device by synthetic HID

So I checked out that commit and found that it shouldn't work either,
since it has the same problem of the current code.

The ACPI video controller device is given an id of ACPI_VIDEO_HID, but
it's only known to Linux ACPI, not ACPICA, so the function provided by
ACPICA acpi_get_devices will not work in this case, as that function will
really check the control method of _HID under the handle, which does not
exist no matter if Linux ACPI has added an id to its device structure or
not. OTOH, the function provided by Linux ACPI acpi_device_hid will see
the added id. In a word, the add of the HID will not affect the ASL
namespace layout of the device node(thus, no _HID control method will
be added to the device node).

It seems that this problem has been found previously by:

commit ff413195e830541afeae469fc866ecd0319abd7e
Author: Alex Hung alex.h...@canonical.com
Date:   Tue Apr 24 16:40:52 2012 +0800

thinkpad-acpi: fix issuing duplicated key events for brightness up/down

The tp_features.bright_acpimode will not be set correctly for brightness
control because ACPI_VIDEO_HID will not be located in ACPI. As a result,
a duplicated key event will always be sent. acpi_video_backlight_support()
is sufficient to detect standard ACPI brightness control.

Signed-off-by: Alex Hung alex.h...@canonical.com
Signed-off-by: Matthew Garrett m...@redhat.com


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


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

2013-09-25 Thread Aaron Lu
On Wed, Sep 25, 2013 at 07:53:13PM +0200, Rafael J. Wysocki wrote:
 On Tuesday, September 24, 2013 05:47:31 PM Aaron Lu wrote:
  According to Matthew Garrett, Windows 8 leaves backlight control up
  to individual graphics drivers rather than making ACPI calls itself.
  There's plenty of evidence to suggest that the Intel driver for
  Windows [8] doesn't use the ACPI interface, including the fact that
  it's broken on a bunch of machines when the OS claims to support
  Windows 8.  The simplest thing to do appears to be to disable the
  ACPI backlight interface on these systems.
  
  So for Win8 systems, if there is native backlight control interface
  registered by GPU driver, ACPI video will not register its own. For
  users who prefer to keep ACPI video's backlight interface, the existing
  kernel cmdline option acpi_backlight=video can be used.
 
 I think the idea is to use the aggressive default for now and we can switch 
 the
 default back to the current behavior before the merge window in case there are
 too many problems with it?

Yes I think so.

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