Re: [Intel-gfx] [git pull] drm for 3.10-rc1

2013-05-03 Thread Josh Boyer
On Fri, May 03, 2013 at 10:40:17PM +0200, Daniel Vetter wrote:
>On Fri, May 3, 2013 at 10:31 PM, Josh Boyer  wrote:
>> OK.  Git bisect tells me this:
>>
>> 57c219633275c7e7413f8bc7be250dc092887458 is the first bad commit
>> commit 57c219633275c7e7413f8bc7be250dc092887458
>> Author: Daniel Vetter 
>> Date:   Thu Apr 4 17:19:37 2013 +0200
>>
>> drm/i915: revert eDP bpp clamping code changes
>
>Yeah, that commit is a bit dubios and for 3.11 we've already planned
>to kick it out. It tries to work around an issue on a funky
>pre-release hw. Does reverting this commit fix your issue?

Yes, seems so.  I reverted it on top of Linus tree as of commit
ce857229e0c3adc2 and things boot normally on the machine after that.

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


Re: [Intel-gfx] [PATCH 2/2]i-g-t: gem_dummy_reloc_loop.c: add vebox test case

2013-05-03 Thread Ben Widawsky
On Fri, May 03, 2013 at 03:55:34PM +0800, Zhong Li wrote:
> Signed-off-by: Zhong Li 
> ---
>  tests/gem_dummy_reloc_loop.c |   21 +
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/gem_dummy_reloc_loop.c b/tests/gem_dummy_reloc_loop.c
> index b67c7d3..b5da577 100644
> --- a/tests/gem_dummy_reloc_loop.c
> +++ b/tests/gem_dummy_reloc_loop.c
> @@ -42,6 +42,8 @@
>  #include "intel_gpu_tools.h"
>  #include "i830_reg.h"
>  
> +#define LOCACL_I915_EXEC_VEBOX (4<<0)
> +

s/LOCACL/LOCAL

>  static drm_intel_bufmgr *bufmgr;
>  struct intel_batchbuffer *batch;
>  static drm_intel_bo *target_buffer;
> @@ -88,14 +90,14 @@ dummy_reloc_loop(int ring)
>  }
>  
>  static void
> -dummy_reloc_loop_random_ring(void)
> +dummy_reloc_loop_random_ring(int num_rings)
>  {
>   int i;
>  
>   srandom(0xdeadbeef);
>  
>   for (i = 0; i < 0x10; i++) {
> - int ring = random() % 3 + 1;
> + int ring = random() % num_rings + 1;
>  
>   if (ring == I915_EXEC_RENDER) {
>   BEGIN_BATCH(4);
> @@ -126,11 +128,13 @@ int main(int argc, char **argv)
>  {
>   int fd;
>   int devid;
> + int num_rings = 1; /*render ring is alwyas available*/

Initializing this to 1 is a bit silly.

>  
>   drmtest_subtest_init(argc, argv);
>  
>   fd = drm_open_any();
>   devid = intel_get_drm_devid(fd);
> + num_rings = gem_get_num_rings(fd);
>   if (!HAS_BLT_RING(devid)) {
>   fprintf(stderr, "not (yet) implemented for pre-snb\n");
>   return 77;
> @@ -179,11 +183,20 @@ int main(int argc, char **argv)
>   }
>   }
>  
> + if (drmtest_run_subtest("vebox")) {
> + if (gem_has_vebox(fd)) {
> + sleep(2);
> + printf("running dummy loop on vebox\n");
> + dummy_reloc_loop(LOCACL_I915_EXEC_VEBOX);
> + printf("dummy loop run on vebox completed\n");
> + }
> + }
> +
>   if (drmtest_run_subtest("mixed")) {
> - if (HAS_BLT_RING(devid) && HAS_BSD_RING(devid)) {
> + if (num_rings > 1) {

You stuck in a small behavioral change here for the test suite which
could potential regress (or highlight previously uncaught bugs)
platforms that don't have VEBOX. The change you've made is fine with me,
but it belongs in the commit message.

>   sleep(2);
>   printf("running dummy loop on random rings\n");
> - dummy_reloc_loop_random_ring();
> + dummy_reloc_loop_random_ring(num_rings);
>   printf("dummy loop run on random rings completed\n");
>   }
>   }
> -- 
> 1.7.9.5
> 

With a commit message explaining the change I mentioned above, I've
merged this to master.

-- 
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 1/2]i-g-t: check kernel enable rings or not

2013-05-03 Thread Ben Widawsky
On Fri, May 03, 2013 at 03:54:48PM +0800, Zhong Li wrote:
> Signed-off-by: Zhong Li 
> 
> 1. add functions check kernel enable a ring or not.
> 2. add function gem_get_num_rings() to check how many rings kernel has enable.
> 3. gem_ring_sync_loop.c will call gem_get_num_rings() directly instead of 
> original static fucntion get_number_rings().

Please wrap commit messages, and place the Signed-off-by at the bottom.

> ---
>  lib/drmtest.c  |   61 
> ++--
>  lib/drmtest.h  |6 -
>  tests/gem_ring_sync_loop.c |   38 +--
>  3 files changed, 59 insertions(+), 46 deletions(-)

> 
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 2ddaff0..a6a988b 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -270,19 +270,64 @@ void gem_set_tiling(int fd, uint32_t handle, int 
> tiling, int stride)
>   assert(st.tiling_mode == tiling);
>  }
>  
> +bool gem_has_enable_ring(int fd,int param)
> +{
> + drm_i915_getparam_t gp;
> + int ret, tmp;
> + memset(&gp, 0, sizeof(gp));
> + 
> + gp.value = &tmp;
> + gp.param = param;
> + 
> + ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> + 
> + if ((ret == 0) && (*gp.value > 0))
> + return true;
> + else
> + return false;
> +}

Whitespace problems again. Again, please fix your editor, or whatever
you must do.

> +
> +bool gem_has_bsd(int fd)
> +{
> +
> + return gem_has_enable_ring(fd,I915_PARAM_HAS_BSD);
> +}
> +
> +bool gem_has_blt(int fd)
> +{
> +
> + return gem_has_enable_ring(fd,I915_PARAM_HAS_BLT);
> +}
> +
>  #define LOCAL_I915_PARAM_HAS_VEBOX 22
> -int gem_has_vebox(int fd)
> +bool gem_has_vebox(int fd)
>  {
> - struct drm_i915_getparam gp;
> - int val;
>  
> - gp.param = LOCAL_I915_PARAM_HAS_VEBOX;
> - gp.value = &val;
> + return gem_has_enable_ring(fd,LOCAL_I915_PARAM_HAS_VEBOX);
> +}
> +
> +int gem_get_num_rings(int fd)
> +{
> + int num_rings = 1;  /* render ring is always available */
> +
> + if (gem_has_bsd(fd))
> + num_rings++;
> + else
> + goto skip;
> +
> + if (gem_has_blt(fd))
> + num_rings++;
> + else
> + goto skip;
> +
> + if (gem_has_vebox(fd))
> + num_rings++;
> + else
> + goto skip;
>  
> - if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp)))
> - return 0;
>  
> - return val != 0;
> +skip:
> + return num_rings;
I know you just copied this from Haihao's original code... the goto is a
bit superfluous.
>  }
>  
>  struct local_drm_i915_gem_cacheing {
> diff --git a/lib/drmtest.h b/lib/drmtest.h
> index f15c074..3549c47 100644
> --- a/lib/drmtest.h
> +++ b/lib/drmtest.h
> @@ -47,8 +47,12 @@ int drm_open_any_master(void);
>  void gem_quiescent_gpu(int fd);
>  
>  /* ioctl wrappers and similar stuff for bare metal testing */
> -int gem_has_vebox(int fd);
>  void gem_set_tiling(int fd, uint32_t handle, int tiling, int stride);
> +bool gem_has_enable_ring(int fd,int param);
> +bool gem_has_bsd(int fd);
> +bool gem_has_blt(int fd);
> +bool gem_has_vebox(int fd);
> +int gem_get_num_rings(int fd);
>  int gem_has_cacheing(int fd);
>  void gem_set_cacheing(int fd, uint32_t handle, int cacheing);
>  int gem_get_cacheing(int fd, uint32_t handle);
> diff --git a/tests/gem_ring_sync_loop.c b/tests/gem_ring_sync_loop.c
> index 501e97c..af40590 100644
> --- a/tests/gem_ring_sync_loop.c
> +++ b/tests/gem_ring_sync_loop.c
> @@ -55,47 +55,11 @@ static drm_intel_bo *target_buffer;
>  #define MI_COND_BATCH_BUFFER_END (0x36<<23 | 1)
>  #define MI_DO_COMPARE(1<<21)
>  
> -static int
> -get_num_rings(int fd)
> -{
> - int num_rings = 1;  /* render ring is always available */
> - drm_i915_getparam_t gp;
> - int ret, tmp;
> -
> - memset(&gp, 0, sizeof(gp));
> - gp.value = &tmp;
> -
> - gp.param = I915_PARAM_HAS_BSD;
> - ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> -
> - if ((ret == 0) && (*gp.value > 0))
> - num_rings++;
> - else
> - goto skip;
> -
> - gp.param = I915_PARAM_HAS_BLT;
> - ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> -
> - if ((ret == 0) && (*gp.value > 0))
> - num_rings++;
> - else
> - goto skip;
> -
> - if (gem_has_vebox(fd))
> - num_rings++;
> - else
> - goto skip;
> -
> -
> -skip:
> - return num_rings;
> -}
> -
>  static void
>  store_dword_loop(int fd)
>  {
>   int i;
> - int num_rings = get_num_rings(fd);
> + int num_rings = gem_get_num_rings(fd);
>  
>   srandom(0xdeadbeef);
>  
> -- 
> 1.7.9.5
> 

With the whitespace fix, I've merged this patch to master.

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

[Intel-gfx] [PATCH] drm/i915: Whitespace fix in gen6_ppgtt_init

2013-05-03 Thread Ben Widawsky
When the recent pde calculation was fixed in:
commit 43b27290dd42b40f3f23f49677a7faa5a4eb1eff
Author: Zhang, Xiong Y 
Date:   Sat Apr 27 09:53:33 2013 +

drm/i915: correct the calculation of first_pd_entry_in_global_pt

It had whitespace errors which I thought Daniel caught and fixed on
merge, but I still see it here on -nightly.

Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 1718936..09b7310 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -263,7 +263,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
/* ppgtt PDEs reside in the global gtt pagetable, which has 512*1024
 * entries. For aliasing ppgtt support we just steal them at the end for
 * now. */
-   first_pd_entry_in_global_pt = gtt_total_entries(dev_priv->gtt);
+   first_pd_entry_in_global_pt = gtt_total_entries(dev_priv->gtt);
 
if (IS_HASWELL(dev)) {
ppgtt->pte_encode = hsw_pte_encode;
-- 
1.8.2.2

___
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: add context into request struct

2013-05-03 Thread Ben Widawsky
On Thu, May 02, 2013 at 04:48:08PM +0300, Mika Kuoppala wrote:
> Storing context reference into request struct
> allows us to inspect context and its associated
> objects when requests are retired.
> 
> Both ppgtt and arb robustness work will need
> this.
> 
> Signed-off-by: Mika Kuoppala 

Both 1&2 are:
Reviewed-by: Ben Widawsky 

You should add your sob to 1, since you modified it (slightly), and
maybe run them both my Chris to make sure he approves as well.

Thanks a lot of reworking the series to this order :D

[snip]

-- 
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] drm/i915: get mode clock when reading the pipe config v2

2013-05-03 Thread Jesse Barnes
On Fri,  3 May 2013 15:34:41 -0700
Jesse Barnes  wrote:

> We need this for comparing modes between configuration changes.
> 
> v2: try harder to calulate non-simple pixel clocks (Daniel)
> call get_clock after getting the encoder config, needed for pixel multiply
> (Jesse)
> 

In reply to danvet's questions:
  - I can move the pixel multiplier into get_config; I thought some of
our encoder regs had a bit that could be used
  - if I do that, I don't need to call get_clock separately; it needs
the pixel multiplier to calculate m/n correctly, so that's why I
had to put it after the encoder get_config

I added pipe config checks for the new mode flags, and I get warnings
now, but I don't want to add one for the clock just yet.  Maybe after
everything is in and working we can add one...

-- 
Jesse Barnes, 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: Kill unused variable warning

2013-05-03 Thread Ben Widawsky
commit b7eab0f52c47c06fc5daa20d40359a97d5d5d3a7
Author: Jesse Barnes 
Date:   Thu May 2 15:30:47 2013 -0700

drm/i915: fix Haswell pfit power well check v2

The above commit got rid of the I915_WRITE making the dev_priv no longer
used. Should be squashed if not too late.

CC: Jesse Barnes 
Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/intel_display.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 78e8494..c0c4dfc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5916,7 +5916,6 @@ static bool ironlake_get_pipe_config(struct intel_crtc 
*crtc,
 
 static void haswell_modeset_global_resources(struct drm_device *dev)
 {
-   struct drm_i915_private *dev_priv = dev->dev_private;
bool enable = false;
struct intel_crtc *crtc;
struct intel_encoder *encoder;
-- 
1.8.2.2

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


[Intel-gfx] [PATCH] drm/i915: get mode clock when reading the pipe config v2

2013-05-03 Thread Jesse Barnes
We need this for comparing modes between configuration changes.

v2: try harder to calulate non-simple pixel clocks (Daniel)
call get_clock after getting the encoder config, needed for pixel multiply
(Jesse)

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/i915_drv.h  |1 +
 drivers/gpu/drm/i915/intel_crt.c |1 +
 drivers/gpu/drm/i915/intel_display.c |   80 +++---
 drivers/gpu/drm/i915/intel_dp.c  |8 
 drivers/gpu/drm/i915/intel_drv.h |3 ++
 drivers/gpu/drm/i915/intel_dvo.c |1 +
 drivers/gpu/drm/i915/intel_hdmi.c|1 +
 drivers/gpu/drm/i915/intel_lvds.c|1 +
 drivers/gpu/drm/i915/intel_sdvo.c|2 +
 9 files changed, 91 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3ac71db..fb3ce07 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -305,6 +305,7 @@ struct drm_i915_display_funcs {
 * fills out the pipe-config with the hw state. */
bool (*get_pipe_config)(struct intel_crtc *,
struct intel_crtc_config *);
+   void (*get_clock)(struct intel_crtc *, struct intel_crtc_config *);
int (*crtc_mode_set)(struct drm_crtc *crtc,
 int x, int y,
 struct drm_framebuffer *old_fb);
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 1370870..b2bfbe4 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -101,6 +101,7 @@ static void intel_crt_get_config(struct intel_encoder 
*encoder,
flags |= DRM_MODE_FLAG_NVSYNC;
 
pipe_config->adjusted_mode.flags |= flags;
+   pipe_config->pixel_multiplier = 1;
 }
 
 static void intel_disable_crt(struct intel_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 7b4005b..a169b8e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -45,6 +45,11 @@ bool intel_pipe_has_type(struct drm_crtc *crtc, int type);
 static void intel_increase_pllclock(struct drm_crtc *crtc);
 static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
 
+static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
+   struct intel_crtc_config *pipe_config);
+static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
+   struct intel_crtc_config *pipe_config);
+
 typedef struct {
int min, max;
 } intel_range_t;
@@ -6902,11 +6907,12 @@ void intel_release_load_detect_pipe(struct 
drm_connector *connector,
 }
 
 /* Returns the clock of the currently programmed mode of the given pipe. */
-static int intel_crtc_clock_get(struct drm_device *dev, struct drm_crtc *crtc)
+static void i9xx_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;
-   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-   int pipe = intel_crtc->pipe;
+   int pipe = pipe_config->cpu_transcoder;
u32 dpll = I915_READ(DPLL(pipe));
u32 fp;
intel_clock_t clock;
@@ -6945,7 +6951,8 @@ static int intel_crtc_clock_get(struct drm_device *dev, 
struct drm_crtc *crtc)
default:
DRM_DEBUG_KMS("Unknown DPLL mode %08x in programmed "
  "mode\n", (int)(dpll & DPLL_MODE_MASK));
-   return 0;
+   pipe_config->adjusted_mode.clock = 0;
+   return;
}
 
/* XXX: Handle the 100Mhz refclk */
@@ -6984,8 +6991,56 @@ static int intel_crtc_clock_get(struct drm_device *dev, 
struct drm_crtc *crtc)
 * i830PllIsValid() because it relies on the xf86_config connector
 * configuration being accurate, which it isn't necessarily.
 */
+   pipe_config->adjusted_mode.clock = clock.dot;
+}
+
+static void ironlake_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;
+   enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
+   int link_freq, repeat;
+   u64 clock;
+   u32 link_m, link_n;
 
-   return clock.dot;
+   /* FIXME: Haswell bits */
+
+   repeat = pipe_config->pixel_multiplier;
+
+   /*
+* The calculation for the data clock is:
+* pixel_clock = ((m/n)*(link_clock * nr_lanes * repeat))/bpp
+* But we want to avoid losing precison if possible, so:
+* pixel_clock = ((m * link_clock * nr_lanes * repeat)/(n*bpp))
+*
+* and the link clock is simpl

[Intel-gfx] [PATCH] drm/i915: get mode clock when reading the pipe config

2013-05-03 Thread Jesse Barnes
We need this for comparing modes between configuration changes.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c |   49 +-
 1 file changed, 43 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 7b4005b..0696e4a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -45,6 +45,11 @@ bool intel_pipe_has_type(struct drm_crtc *crtc, int type);
 static void intel_increase_pllclock(struct drm_crtc *crtc);
 static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
 
+static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
+   struct intel_crtc_config *pipe_config);
+static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
+   struct intel_crtc_config *pipe_config);
+
 typedef struct {
int min, max;
 } intel_range_t;
@@ -4981,6 +4986,8 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 
intel_get_pipe_timings(crtc, pipe_config);
 
+   i9xx_crtc_clock_get(crtc, pipe_config);
+
return true;
 }
 
@@ -5900,6 +5907,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc 
*crtc,
 
intel_get_pipe_timings(crtc, pipe_config);
 
+   ironlake_crtc_clock_get(crtc, pipe_config);
+
return true;
 }
 
@@ -6048,6 +6057,8 @@ static bool haswell_get_pipe_config(struct intel_crtc 
*crtc,
 
intel_get_pipe_timings(crtc, pipe_config);
 
+   ironlake_crtc_clock_get(crtc, pipe_config);
+
return true;
 }
 
@@ -6902,11 +6913,12 @@ void intel_release_load_detect_pipe(struct 
drm_connector *connector,
 }
 
 /* Returns the clock of the currently programmed mode of the given pipe. */
-static int intel_crtc_clock_get(struct drm_device *dev, struct drm_crtc *crtc)
+static void i9xx_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;
-   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-   int pipe = intel_crtc->pipe;
+   int pipe = pipe_config->cpu_transcoder;
u32 dpll = I915_READ(DPLL(pipe));
u32 fp;
intel_clock_t clock;
@@ -6945,7 +6957,8 @@ static int intel_crtc_clock_get(struct drm_device *dev, 
struct drm_crtc *crtc)
default:
DRM_DEBUG_KMS("Unknown DPLL mode %08x in programmed "
  "mode\n", (int)(dpll & DPLL_MODE_MASK));
-   return 0;
+   pipe_config->adjusted_mode.clock = 0;
+   return;
}
 
/* XXX: Handle the 100Mhz refclk */
@@ -6984,8 +6997,28 @@ static int intel_crtc_clock_get(struct drm_device *dev, 
struct drm_crtc *crtc)
 * i830PllIsValid() because it relies on the xf86_config connector
 * configuration being accurate, which it isn't necessarily.
 */
+   pipe_config->adjusted_mode.clock = clock.dot;
+}
+
+static void ironlake_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;
+   enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
+   int clock;
+   u32 link_m;
+
+   /*
+* PCH platforms make this easy: we can just use the LINK_M1 reg.
+* Note: this may be the pixel clock for a fitted mode, in which
+* case it won't match the native mode clock.  That means we won't be
+* able to do a simple flip in the fastboot case.
+*/
+   link_m = I915_READ(PIPE_LINK_M1(cpu_transcoder));
+   clock = link_m;
 
-   return clock.dot;
+   pipe_config->adjusted_mode.clock = clock;
 }
 
 /** Returns the currently programmed mode of the given pipe. */
@@ -6996,6 +7029,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct 
drm_device *dev,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
struct drm_display_mode *mode;
+   struct intel_crtc_config pipe_config;
int htot = I915_READ(HTOTAL(cpu_transcoder));
int hsync = I915_READ(HSYNC(cpu_transcoder));
int vtot = I915_READ(VTOTAL(cpu_transcoder));
@@ -7005,7 +7039,10 @@ struct drm_display_mode *intel_crtc_mode_get(struct 
drm_device *dev,
if (!mode)
return NULL;
 
-   mode->clock = intel_crtc_clock_get(dev, crtc);
+   pipe_config.cpu_transcoder = intel_crtc->pipe;
+   i9xx_crtc_clock_get(intel_crtc, &pipe_config);
+
+   mode->clock = pipe_config.adjusted_mode.clock;
mode->hdisplay = (htot & 0x) + 1;
mode->htotal = ((htot & 0x) >> 16) + 1;

Re: [Intel-gfx] [git pull] drm for 3.10-rc1

2013-05-03 Thread Daniel Vetter
On Fri, May 3, 2013 at 10:31 PM, Josh Boyer  wrote:
> OK.  Git bisect tells me this:
>
> 57c219633275c7e7413f8bc7be250dc092887458 is the first bad commit
> commit 57c219633275c7e7413f8bc7be250dc092887458
> Author: Daniel Vetter 
> Date:   Thu Apr 4 17:19:37 2013 +0200
>
> drm/i915: revert eDP bpp clamping code changes

Yeah, that commit is a bit dubios and for 3.11 we've already planned
to kick it out. It tries to work around an issue on a funky
pre-release hw. Does reverting this commit fix your issue?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [git pull] drm for 3.10-rc1

2013-05-03 Thread Josh Boyer
On Fri, May 03, 2013 at 12:57:20PM -0400, Josh Boyer wrote:
>On Fri, May 03, 2013 at 06:08:52PM +0200, Daniel Vetter wrote:
>>On Fri, May 3, 2013 at 4:39 PM, Josh Boyer  wrote:
>>> On Fri, May 03, 2013 at 02:25:57AM +0100, Dave Airlie wrote:
The following changes since commit b6a9b7f6b1f21735a7456d534dc0e68e61359d2c:

  mm: prevent mmap_cache race in find_vma() (2013-04-04 11:46:28 -0700)

are available in the git repository at:

  git://people.freedesktop.org/~airlied/linux.git drm-next

for you to fetch changes up to 307b9c022720f9de90d58e51743e01e9a42aec59:

  qxl: update to new idr interfaces. (2013-05-03 10:37:20 +1000)


>>>
>>> So something in this batch of commits breaks a Macbook Pro Retina I have
>>> sitting here.  If I boot a Fedora kernel based on  Linux
>>> v3.9-8153-g5a148af, things boot up as they did previously with 3.9.0 and
>>> are generally working fine.  If I boot with a kernel based on Linux
>>> v3.9-8933-gce85722, it will boot but as soon as plymouth starts, I get
>>> nothing but static on the screen (like 1950s TV static).
>>>
>>> This machine is using i915 graphics, so I booted with nomodeset and did
>>> the 'modprobe drm debug=15; modprobe i915 modeset=1' trick and it
>>> repeated the failure with that.  I've gathered a bunch of data like
>>> dmesg, an intel_reg_snapshot, etc.  It's all attached below.
>>>
>>> I'll start bisecting to see if I can narrow down the commit that broke
>>> things, but I thought I would give a heads up now in case anyone has any
>>> ideas.
>>
>>Looked through the log files and didn't see a clue. And usually DP
>>tends to fail pretty noisily. So I think the bisect result would be
>>interestin. Meanwhile:
>
>Yeah, working on that.

OK.  Git bisect tells me this:

57c219633275c7e7413f8bc7be250dc092887458 is the first bad commit
commit 57c219633275c7e7413f8bc7be250dc092887458
Author: Daniel Vetter 
Date:   Thu Apr 4 17:19:37 2013 +0200

drm/i915: revert eDP bpp clamping code changes

The behaviour around handling the eDP bpp value from vbt has been
slightly changed in

commit 3600836585e3fdef0a1410d63fe5ce4015007aac
Author: Daniel Vetter 
Date:   Wed Mar 27 00:44:59 2013 +0100

drm/i915: convert DP autodither code to new infrastructure

The old behaviour was that we used the plane's bpp (usually 24bpp) for
computing the dp link bw, but set up the pipe with the bpp value from
vbt if available. This takes the vbt bpp override into account even
for the dp link bw configuration.

On Paulo's hsw machine this resulted in a slower link clock and a
black screen - but the mode actually /should/ fit even with the lower
clock. Until we've cleared up simply stay bug-for-bug compatible with
the old code.

While at it, also restore a debug message lost in:

commit 4e53c2e010e531b4a014692199e978482d471c7e
Author: Daniel Vetter 
Date:   Wed Mar 27 00:44:58 2013 +0100

drm/i915: precompute pipe bpp before touching the hw

Cc: Paulo Zanoni 
Reviewed-by: Paulo Zanoni 
Tested-by: Paulo Zanoni 
Signed-off-by: Daniel Vetter 

:04 04 a7529c568073885302e5ca03cce5bd224e244b57 
d5d860a0d4b04ad98d77abb1df774c1448bd1697 M  drivers

The bisect log is below.  I can try to revert it directly on top of
Linus' latest tree later.

>>- do you have drm.debug=0xe dmesg for a working kernel, too?
>
>No, but I can try and get one.

I haven't gotten to this quite yet.  I'll try to get to it over the
weekend.

josh

[jwboyer@obiwan linux]$ git bisect log
git bisect start
# good: [5a148af66932c31814e263366094b5812210b501] Merge branch 'next' of 
git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc
git bisect good 5a148af66932c31814e263366094b5812210b501
# bad: [ce857229e0c3adc211944a13a5579ef84fd7b4af] ipc: fix GETALL/IPC_RM race 
for sysv semaphores
git bisect bad ce857229e0c3adc211944a13a5579ef84fd7b4af
# bad: [4ed108352d9b60a723a5071ed05e722826c2b72f] drm/radeon: put UVD PLLs in 
bypass mode
git bisect bad 4ed108352d9b60a723a5071ed05e722826c2b72f
# bad: [dc652f90e088798bfa31f496ba994ddadd5d5680] drm/i915: ensure single 
initialization and cleanup of backlight device
git bisect bad dc652f90e088798bfa31f496ba994ddadd5d5680
# good: [bb60b9695ced58768ba05b2d88fb4ee815df18f4] drm/i915: emit a hotplug 
event on resume
git bisect good bb60b9695ced58768ba05b2d88fb4ee815df18f4
# good: [8b47047bd103c9fdb50440790a2ef17fa69a35c4] drm/i915: rip out superflous 
is_dp&is_cpu_edp tracking
git bisect good 8b47047bd103c9fdb50440790a2ef17fa69a35c4
# bad: [4615d4c9e27eda42c3e965f208a4b4065841498c] drm/i915: Use MLC (l3$) for 
context objects
git bisect bad 4615d4c9e27eda42c3e965f208a4b4065841498c
# bad: [40c7ead980945ac96eadbd6d99b050458d797e2b] drm/i915: PCH_NOP
git bisect bad 40c7ead980945ac96eadbd6d99b050458d797e2b
# good: [9c8e09b7a551fc81842a2

[Intel-gfx] [PATCH] drm/i915: fix panel fitting on LVDS on ILK+ v2

2013-05-03 Thread Jesse Barnes
This regression was introduced in:

commit b074cec8c652f2d273907a4b35239b4766c894ac
Author: Jesse Barnes 
Date:   Thu Apr 25 12:55:02 2013 -0700

drm/i915: move PCH pfit controls into pipe_config

In refactoring this, it was only applied to eDP, which is incorrect.  In
fact, if we ever use the panel fitter to deal with overscan on HDMI,
we'll need to extend it again, so just drop the conditional altogether.

v2: drop check for eDP since we can use the fitter in any config (Daniel)

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index aa99b4d..7b4005b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3239,8 +3239,7 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc)
struct drm_i915_private *dev_priv = dev->dev_private;
int pipe = crtc->pipe;
 
-   if (crtc->config.pch_pfit.size &&
-   intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_EDP)) {
+   if (crtc->config.pch_pfit.size) {
/* Force use of hard-coded filter coefficients
 * as some pre-programmed values are broken,
 * e.g. x201.
-- 
1.7.9.5

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


[Intel-gfx] [PATCH 9/9] drm/i915: set FORCE_ARB_IDLE_PLANES workaround

2013-05-03 Thread Paulo Zanoni
From: Paulo Zanoni 

Commit 1544d9d57396d5c0c6b7644ed5ae1f4d6caad07a added a workaround
inside haswell_init_clock_gating and mentioned it is "a workaround for
early silicon revisions and should be removed later". This workaround
is documented in bit 31 of PRI_CTL. I asked Arthur and he mentioned
that setting FORCE_ARB_IDLE_PLANES replaces that workaround for the
newer machines. So use the new one.

Also notice that there's still another workaround for PRI_CTL that
involves WM_DBG, but it's not the one we're reverting. And notice that
we were previously setting WM_DBG_DISALLOW_MULTIPIPE_LP which disables
the LP watermarks when more than one pipe is used, and we really don't
want this because we need the LP watermarks if we want to reach deeper
PC states.

Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/i915_reg.h |3 +++
 drivers/gpu/drm/i915/intel_pm.c |   10 ++
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index aec569f..5879f23 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3697,6 +3697,9 @@
 # define CHICKEN3_DGMG_REQ_OUT_FIX_DISABLE (1 << 5)
 # define CHICKEN3_DGMG_DONE_FIX_DISABLE(1 << 2)
 
+#define CHICKEN_PAR1_1 0x42080
+#define  FORCE_ARB_IDLE_PLANES (1 << 14)
+
 #define DISP_ARB_CTL   0x45000
 #define  DISP_TILE_SURFACE_SWIZZLING   (1<<13)
 #define  DISP_FBC_WM_DIS   (1<<15)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b56de92..2297476 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4042,14 +4042,8 @@ static void haswell_init_clock_gating(struct drm_device 
*dev)
/* WaSwitchSolVfFArbitrationPriority */
I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
 
-   /* XXX: This is a workaround for early silicon revisions and should be
-* removed later.
-*/
-   I915_WRITE(WM_DBG,
-   I915_READ(WM_DBG) |
-   WM_DBG_DISALLOW_MULTIPLE_LP |
-   WM_DBG_DISALLOW_SPRITE |
-   WM_DBG_DISALLOW_MAXFIFO);
+   I915_WRITE(CHICKEN_PAR1_1,
+  I915_READ(CHICKEN_PAR1_1) | FORCE_ARB_IDLE_PLANES);
 
lpt_init_clock_gating(dev);
 }
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 8/9] drm/i915: MCH_SSKPD is a 64 bit register on Haswell

2013-05-03 Thread Paulo Zanoni
From: Paulo Zanoni 

And the SNB_READ_WM0_LATENCY macro is not valid anymore because we
have the "New WM0" at 63:56, so the "Old WM0" could maybe be zero if
the new one is not zero.

Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/intel_pm.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 59bac2e..b56de92 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4491,7 +4491,7 @@ void intel_init_pm(struct drm_device *dev)
}
dev_priv->display.init_clock_gating = 
ivybridge_init_clock_gating;
} else if (IS_HASWELL(dev)) {
-   if (SNB_READ_WM0_LATENCY()) {
+   if (I915_READ64(MCH_SSKPD)) {
dev_priv->display.update_wm = haswell_update_wm;
dev_priv->display.update_sprite_wm = 
sandybridge_update_sprite_wm;
} else {
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 7/9] drm/i915: set the IPS linetime watermark

2013-05-03 Thread Paulo Zanoni
From: Paulo Zanoni 

Remove the "placeholder" comment and set the actual value described by
the specification. We still don't enable IPS, but it won't hurt to
already have the value set here.

While at it, fully set the register value instead of just masking the
values we're changing.

Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/intel_pm.c |   22 +++---
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3ca020c..59bac2e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2022,7 +2022,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct 
drm_crtc *crtc)
enum pipe pipe = intel_crtc->pipe;
struct drm_display_mode *mode = &intel_crtc->config.adjusted_mode;
int target_clock;
-   u32 temp;
+   u32 linetime, ips_linetime;
 
if (!intel_crtc_active(crtc)) {
I915_WRITE(PIPE_WM_LINETIME(pipe), 0);
@@ -2034,24 +2034,16 @@ haswell_update_linetime_wm(struct drm_device *dev, 
struct drm_crtc *crtc)
else
target_clock = intel_crtc->config.adjusted_mode.clock;
 
-   temp = I915_READ(PIPE_WM_LINETIME(pipe));
-   temp &= ~PIPE_WM_LINETIME_MASK;
-
/* The WM are computed with base on how long it takes to fill a single
 * row at the given clock rate, multiplied by 8.
 * */
-   temp |= PIPE_WM_LINETIME_TIME(
-   DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8, target_clock));
-
-   /* IPS watermarks are only used by pipe A, and are ignored by
-* pipes B and C.  They are calculated similarly to the common
-* linetime values, except that we are using CD clock frequency
-* in MHz instead of pixel rate for the division.
-*
-* This is a placeholder for the IPS watermark calculation code.
-*/
+   linetime = DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8, target_clock);
+   ips_linetime = DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8,
+intel_ddi_get_cdclk_freq(dev_priv));
 
-   I915_WRITE(PIPE_WM_LINETIME(pipe), temp);
+   I915_WRITE(PIPE_WM_LINETIME(pipe),
+  PIPE_WM_LINETIME_IPS_LINETIME(ips_linetime) |
+  PIPE_WM_LINETIME_TIME(linetime));
 }
 
 static void haswell_update_wm(struct drm_device *dev)
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 6/9] drm/i915: make intel_ddi_get_cdclk_freq return values in KHz

2013-05-03 Thread Paulo Zanoni
From: Paulo Zanoni 

With this, that 338 can finally become the correct 337500.

Due to the change we need to adjust the intel_dp_aux_ch function to
set the correct value, so adjust the division and also use
DIV_ROUND_CLOSEST instead of the old "round down" behavior because the
spec says the value "should be programmed to get as close as possible
to the ideal rate of 2MHz".

Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/intel_ddi.c |   10 +-
 drivers/gpu/drm/i915/intel_dp.c  |3 ++-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 21fb852..e5b1b63 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1335,14 +1335,14 @@ static void intel_disable_ddi(struct intel_encoder 
*intel_encoder)
 int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv)
 {
if (I915_READ(HSW_FUSE_STRAP) & HSW_CDCLK_LIMIT)
-   return 450;
+   return 45;
else if ((I915_READ(LCPLL_CTL) & LCPLL_CLK_FREQ_MASK) ==
 LCPLL_CLK_FREQ_450)
-   return 450;
+   return 45;
else if (IS_ULT(dev_priv->dev))
-   return 338;
+   return 337500;
else
-   return 540;
+   return 54;
 }
 
 void intel_ddi_pll_init(struct drm_device *dev)
@@ -1355,7 +1355,7 @@ void intel_ddi_pll_init(struct drm_device *dev)
 * Don't even try to turn it on.
 */
 
-   DRM_DEBUG_KMS("CDCLK running at %dMHz\n",
+   DRM_DEBUG_KMS("CDCLK running at %dKHz\n",
  intel_ddi_get_cdclk_freq(dev_priv));
 
if (val & LCPLL_CD_SOURCE_FCLK)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a293523..3df1383 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -346,7 +346,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 */
if (is_cpu_edp(intel_dp)) {
if (HAS_DDI(dev))
-   aux_clock_divider = intel_ddi_get_cdclk_freq(dev_priv) 
>> 1;
+   aux_clock_divider = DIV_ROUND_CLOSEST(
+   intel_ddi_get_cdclk_freq(dev_priv), 2000);
else if (IS_VALLEYVIEW(dev))
aux_clock_divider = 100;
else if (IS_GEN6(dev) || IS_GEN7(dev))
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 5/9] drm/i915: use the correct clock when calculating linetime watermarks

2013-05-03 Thread Paulo Zanoni
From: Paulo Zanoni 

If we're using DP/eDP, adjusted_mode->clock may be just the port link
clock, but we also can't use mode->clock because it's wrong when we're
using the using panel fitter.

Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/intel_pm.c |8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8468b40..3ca020c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2021,6 +2021,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct 
drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
enum pipe pipe = intel_crtc->pipe;
struct drm_display_mode *mode = &intel_crtc->config.adjusted_mode;
+   int target_clock;
u32 temp;
 
if (!intel_crtc_active(crtc)) {
@@ -2028,6 +2029,11 @@ haswell_update_linetime_wm(struct drm_device *dev, 
struct drm_crtc *crtc)
return;
}
 
+   if (intel_crtc->config.pixel_target_clock)
+   target_clock = intel_crtc->config.pixel_target_clock;
+   else
+   target_clock = intel_crtc->config.adjusted_mode.clock;
+
temp = I915_READ(PIPE_WM_LINETIME(pipe));
temp &= ~PIPE_WM_LINETIME_MASK;
 
@@ -2035,7 +2041,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct 
drm_crtc *crtc)
 * row at the given clock rate, multiplied by 8.
 * */
temp |= PIPE_WM_LINETIME_TIME(
-   DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8, mode->clock));
+   DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8, target_clock));
 
/* IPS watermarks are only used by pipe A, and are ignored by
 * pipes B and C.  They are calculated similarly to the common
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 4/9] drm/i915: fix haswell linetime watermarks calculation

2013-05-03 Thread Paulo Zanoni
From: Paulo Zanoni 

Move the "*8"  calculation to the left side so we don't propagate
rounding errors. Also use DIV_ROUND_CLOSEST because that's what the
spec says we need to do.

Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/intel_pm.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4cc5f99..8468b40 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2035,7 +2035,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct 
drm_crtc *crtc)
 * row at the given clock rate, multiplied by 8.
 * */
temp |= PIPE_WM_LINETIME_TIME(
-   ((mode->htotal * 1000) / mode->clock) * 8);
+   DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8, mode->clock));
 
/* IPS watermarks are only used by pipe A, and are ignored by
 * pipes B and C.  They are calculated similarly to the common
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 3/9] drm/i915: use the mode->htotal to calculate linetime watermarks

2013-05-03 Thread Paulo Zanoni
From: Paulo Zanoni 

... instead of mode->crtc_display. The spec says "pipe horizontal
total number of pixels" and the "Haswell Watermark Calculator" tool
uses the "Pipe H Total" instead of "Pipe H Src" as the value.

Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/intel_pm.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d056bc9..4cc5f99 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2035,7 +2035,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct 
drm_crtc *crtc)
 * row at the given clock rate, multiplied by 8.
 * */
temp |= PIPE_WM_LINETIME_TIME(
-   ((mode->crtc_hdisplay * 1000) / mode->clock) * 8);
+   ((mode->htotal * 1000) / mode->clock) * 8);
 
/* IPS watermarks are only used by pipe A, and are ignored by
 * pipes B and C.  They are calculated similarly to the common
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 2/9] drm/i915: fix linetime_watermarks code

2013-05-03 Thread Paulo Zanoni
From: Paulo Zanoni 

The spec says the linetime watermarks must be programmed before
enabling any display low power watermarks, but we're currently
updating the linetime watermarks after we call intel_update_watermarks
(and only at crtc_mode_set, not at crtc_{enable,disable}). So IMHO the
best way guarantee the linetime watermarks will be updated before the
low power watermarks is inside the update_wm function, because it's
the function that enables low power watermarks. And since Haswell is
the only platform that has linetime watermarks, let's completely kill
the "intel_update_linetime_watermarks" abstraction and just use the
intel_update_watermarks abstraction by creating haswell_update_wm.

For now haswell_update_wm is still calling sandybridge_update_wm, but
in the future I plan to implement a function specific to Haswell.

Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/i915_drv.h  |2 --
 drivers/gpu/drm/i915/intel_display.c |2 --
 drivers/gpu/drm/i915/intel_drv.h |2 --
 drivers/gpu/drm/i915/intel_pm.c  |   37 ++
 4 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5160e1e..f9864c7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -316,8 +316,6 @@ struct drm_i915_display_funcs {
void (*update_wm)(struct drm_device *dev);
void (*update_sprite_wm)(struct drm_device *dev, int pipe,
 uint32_t sprite_width, int pixel_size);
-   void (*update_linetime_wm)(struct drm_device *dev, int pipe,
-struct drm_display_mode *mode);
void (*modeset_global_resources)(struct drm_device *dev);
/* Returns the active state of the crtc, and if the crtc is active,
 * fills out the pipe-config with the hw state. */
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 3e742f9..ab5690b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6013,8 +6013,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 
intel_update_watermarks(dev);
 
-   intel_update_linetime_watermarks(dev, pipe, adjusted_mode);
-
return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ae73631..f677af6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -732,8 +732,6 @@ extern void intel_update_watermarks(struct drm_device *dev);
 extern void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
   uint32_t sprite_width,
   int pixel_size);
-extern void intel_update_linetime_watermarks(struct drm_device *dev, int pipe,
-struct drm_display_mode *mode);
 
 extern unsigned long intel_gen4_compute_page_offset(int *x, int *y,
unsigned int tiling_mode,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1afaec4..d056bc9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2015,12 +2015,19 @@ static void ivybridge_update_wm(struct drm_device *dev)
 }
 
 static void
-haswell_update_linetime_wm(struct drm_device *dev, int pipe,
-struct drm_display_mode *mode)
+haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+   enum pipe pipe = intel_crtc->pipe;
+   struct drm_display_mode *mode = &intel_crtc->config.adjusted_mode;
u32 temp;
 
+   if (!intel_crtc_active(crtc)) {
+   I915_WRITE(PIPE_WM_LINETIME(pipe), 0);
+   return;
+   }
+
temp = I915_READ(PIPE_WM_LINETIME(pipe));
temp &= ~PIPE_WM_LINETIME_MASK;
 
@@ -2041,6 +2048,20 @@ haswell_update_linetime_wm(struct drm_device *dev, int 
pipe,
I915_WRITE(PIPE_WM_LINETIME(pipe), temp);
 }
 
+static void haswell_update_wm(struct drm_device *dev)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct drm_crtc *crtc;
+   enum pipe pipe;
+
+   for_each_pipe(pipe) {
+   crtc = dev_priv->pipe_to_crtc_mapping[pipe];
+   haswell_update_linetime_wm(dev, crtc);
+   }
+
+   sandybridge_update_wm(dev);
+}
+
 static bool
 sandybridge_compute_sprite_wm(struct drm_device *dev, int plane,
  uint32_t sprite_width, int pixel_size,
@@ -2236,15 +2257,6 @@ void intel_update_watermarks(struct drm_device *dev)
dev_priv->display.update_wm(dev);
 }
 
-void intel_update_linetime_watermarks(struct drm_device *dev,
-   int pipe, struct drm_display_mode *mode)
-{
-   struct d

[Intel-gfx] [PATCH 1/9] drm/i915: ILK, SNB and IVB don't have linetime watermarks

2013-05-03 Thread Paulo Zanoni
From: Paulo Zanoni 

So don't call intel_update_linetime_watermarks from
ironlake_crtc_mode_set. Only Haswell has these watermarks.

Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/intel_display.c |2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 9af3ec2..3e742f9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5863,8 +5863,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
intel_update_watermarks(dev);
 
-   intel_update_linetime_watermarks(dev, pipe, adjusted_mode);
-
return ret;
 }
 
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 0/9] Haswell watermarks, round 1

2013-05-03 Thread Paulo Zanoni
From: Paulo Zanoni 

Hi

All our Haswell watermarks code does not follow our specifications, and this
series only does some small fixes and also fixes the linetime watermarks. For
the other watermarks we're still calling the old sandybridge update_wm
functions, so the next patches will implement Haswell-specific functions and use
them, also using the linetime watermarks fixed in this series. Consider this
series as a warm-up for the next ones :)

Cheers,
Paulo

Paulo Zanoni (9):
  drm/i915: ILK, SNB and IVB don't have linetime watermarks
  drm/i915: fix linetime_watermarks code
  drm/i915: use the mode->htotal to calculate linetime watermarks
  drm/i915: fix haswell linetime watermarks calculation
  drm/i915: use the correct clock when calculating linetime watermarks
  drm/i915: make intel_ddi_get_cdclk_freq return values in KHz
  drm/i915: set the IPS linetime watermark
  drm/i915: MCH_SSKPD is a 64 bit register on Haswell
  drm/i915: set FORCE_ARB_IDLE_PLANES workaround

 drivers/gpu/drm/i915/i915_drv.h  |2 -
 drivers/gpu/drm/i915/i915_reg.h  |3 ++
 drivers/gpu/drm/i915/intel_ddi.c |   10 ++---
 drivers/gpu/drm/i915/intel_display.c |4 --
 drivers/gpu/drm/i915/intel_dp.c  |3 +-
 drivers/gpu/drm/i915/intel_drv.h |2 -
 drivers/gpu/drm/i915/intel_pm.c  |   73 ++
 7 files changed, 48 insertions(+), 49 deletions(-)

-- 
1.7.10.4

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


Re: [Intel-gfx] [PATCH] drm/i915: fix panel fitting on LVDS on ILK+

2013-05-03 Thread Jesse Barnes
On Fri, 3 May 2013 22:15:27 +0200
Daniel Vetter  wrote:

> On Fri, May 3, 2013 at 10:03 PM, Jesse Barnes  
> wrote:
> > In refactoring this, it was only applied to eDP, which is incorrect.  In
> > fact, if we ever use the panel fitter to deal with overscan on HDMI,
> > we'll need to extend it again.
> >
> > Signed-off-by: Jesse Barnes 
> 
> Citation of regressing commit is missing. Also, since the pfit is
> per-pipe on ilk+, shouldn't we just drop this?

Drop the conditional?  Sure if you want...

-- 
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: fix panel fitting on LVDS on ILK+

2013-05-03 Thread Daniel Vetter
On Fri, May 3, 2013 at 10:03 PM, Jesse Barnes  wrote:
> In refactoring this, it was only applied to eDP, which is incorrect.  In
> fact, if we ever use the panel fitter to deal with overscan on HDMI,
> we'll need to extend it again.
>
> Signed-off-by: Jesse Barnes 

Citation of regressing commit is missing. Also, since the pfit is
per-pipe on ilk+, shouldn't we just drop this?
-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: hw state readout&check support for cpu_transcoder

2013-05-03 Thread Daniel Vetter
This allows us to drop a bunch of ugly hacks and finally implement
what

commit cc464b2a17c59adedbdc02cc54341d630354edc3
Author: Paulo Zanoni 
Date:   Fri Jan 25 16:59:16 2013 -0200

drm/i915: set TRANSCODER_EDP even earlier

tried to achieve, but that was reverted again in

commit bba2181c49f1dddf8b592804a1b53cc1a3cf408a
Author: Daniel Vetter 
Date:   Fri Mar 22 10:53:40 2013 +0100

Revert "drm/i915: set TRANSCODER_EDP even earlier"

Now we should always have a consistent cpu_transcoder in the
pipe_config.

Cc: Paulo Zanoni 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_ddi.c |  4 ++
 drivers/gpu/drm/i915/intel_display.c | 90 +---
 2 files changed, 37 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 3ff4de6..c1a5669 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1448,9 +1448,13 @@ static bool intel_ddi_compute_config(struct 
intel_encoder *encoder,
 struct intel_crtc_config *pipe_config)
 {
int type = encoder->type;
+   int port = intel_ddi_get_encoder_port(encoder);
 
WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown 
output!\n");
 
+   if (port == PORT_A)
+   pipe_config->cpu_transcoder = TRANSCODER_EDP;
+
if (type == INTEL_OUTPUT_HDMI)
return intel_hdmi_compute_config(encoder, pipe_config);
else
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 3b7fae1..ac9ad1a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3566,12 +3566,6 @@ static void ironlake_crtc_off(struct drm_crtc *crtc)
 
 static void haswell_crtc_off(struct drm_crtc *crtc)
 {
-   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-   /* Stop saying we're using TRANSCODER_EDP because some other CRTC might
-* start using it. */
-   intel_crtc->config.cpu_transcoder = (enum transcoder) intel_crtc->pipe;
-
intel_ddi_put_crtc_pll(crtc);
 }
 
@@ -5035,6 +5029,8 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
struct drm_i915_private *dev_priv = dev->dev_private;
uint32_t tmp;
 
+   pipe_config->cpu_transcoder = crtc->pipe;
+
tmp = I915_READ(PIPECONF(crtc->pipe));
if (!(tmp & PIPECONF_ENABLE))
return false;
@@ -5786,8 +5782,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
WARN(!(HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)),
 "Unexpected PCH type %d\n", INTEL_PCH_TYPE(dev));
 
-   intel_crtc->config.cpu_transcoder = pipe;
-
ok = ironlake_compute_clocks(crtc, adjusted_mode, &clock,
 &has_reduced_clock, &reduced_clock);
if (!ok) {
@@ -5910,6 +5904,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc 
*crtc,
struct drm_i915_private *dev_priv = dev->dev_private;
uint32_t tmp;
 
+   pipe_config->cpu_transcoder = crtc->pipe;
+
tmp = I915_READ(PIPECONF(crtc->pipe));
if (!(tmp & PIPECONF_ENABLE))
return false;
@@ -5985,11 +5981,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
num_connectors++;
}
 
-   if (is_cpu_edp)
-   intel_crtc->config.cpu_transcoder = TRANSCODER_EDP;
-   else
-   intel_crtc->config.cpu_transcoder = pipe;
-
/* We are not sure yet this won't happen. */
WARN(!HAS_PCH_LPT(dev), "Unexpected PCH type %d\n",
 INTEL_PCH_TYPE(dev));
@@ -6045,14 +6036,36 @@ static bool haswell_get_pipe_config(struct intel_crtc 
*crtc,
 {
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
-   enum transcoder cpu_transcoder = crtc->config.cpu_transcoder;
uint32_t tmp;
 
+   pipe_config->cpu_transcoder = crtc->pipe;
+   tmp = (TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
+   if (tmp & TRANS_DDI_FUNC_ENABLE) {
+   enum pipe trans_edp_pipe;
+   switch (tmp & TRANS_DDI_EDP_INPUT_MASK) {
+   default:
+   WARN(1, "unknown pipe linked to edp transcoder\n");
+   case TRANS_DDI_EDP_INPUT_A_ONOFF:
+   case TRANS_DDI_EDP_INPUT_A_ON:
+   trans_edp_pipe = PIPE_A;
+   break;
+   case TRANS_DDI_EDP_INPUT_B_ONOFF:
+   trans_edp_pipe = PIPE_A;
+   break;
+   case TRANS_DDI_EDP_INPUT_C_ONOFF:
+   trans_edp_pipe = PIPE_A;
+   break;
+   }
+
+   if (trans_edp_pipe == crtc->pipe)
+   pipe_config->cpu_transcoder = TRANSCODER_EDP;
+   }
+
if (!intel_using_power_well(dev_priv->dev) &&
-   cpu_transcoder != TRANSCODER_EDP)
+

[Intel-gfx] [PATCH] drm/i915: fix panel fitting on LVDS on ILK+

2013-05-03 Thread Jesse Barnes
In refactoring this, it was only applied to eDP, which is incorrect.  In
fact, if we ever use the panel fitter to deal with overscan on HDMI,
we'll need to extend it again.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index aa99b4d..f9894cf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3240,7 +3240,8 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc)
int pipe = crtc->pipe;
 
if (crtc->config.pch_pfit.size &&
-   intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_EDP)) {
+   (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_EDP) ||
+intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS))) {
/* Force use of hard-coded filter coefficients
 * as some pre-programmed values are broken,
 * e.g. x201.
-- 
1.7.9.5

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


[Intel-gfx] Updated testing tree

2013-05-03 Thread Daniel Vetter
Hi all,

Merge windows opens, so we'll start with the first feature batch for 3.11.
Highlights (part of it already in the previous testing cycle, but
postponed for 3.11):
- fixes for the gmch modeset sequence
- a bit of OCD around plane/pipe usage (Ville)
- vlv turbo support (Jesse)
- tons of vlv modeset fixes (Jesse et al.)
- vlv pte write fixes (Kenneth Graunke)
- hpd filtering to avoid costly probes on unaffected outputs (Egbert Eich)
- intel dev_info cleanups and refactorings (Damien)
- vlv rc6 support (Jesse)
- random pile of fixes around non-24bpp modes handling
- asle/opregion cleanups and locking fixes (Jani)
- dp dpll refactoring
- improvements for reduced_clock computation on g4x/ilk+
- pfit state refactored to use pipe_config (Jesse)
- lots more computed modeset state moved to pipe_config, including readout
  and cross-check support
- fdi auto-dithering for ivb B/C links, using the neat pipe_config
  improvements
- drm_rect helpers plus sprite clipping fixes (Ville)
- hw context refcounting (Mika + Ben)

Happy testing!

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


Re: [Intel-gfx] [PATCH 4/4] drm/i915: BIOS and power context stolen mem handling for VLV

2013-05-03 Thread Ben Widawsky
On Thu, May 02, 2013 at 10:48:10AM -0700, Jesse Barnes wrote:
> But we need to get the right stolen base and make pre-allocated objects
> for BIOS stuff so we don't clobber it.  If the BIOS hasn't allocated a
> power context, we allocate one here too, from stolen space as required
> by the docs.
> 
> Signed-off-by: Jesse Barnes 
> ---
>  drivers/gpu/drm/i915/i915_drv.h|2 +
>  drivers/gpu/drm/i915/i915_gem_stolen.c |   78 
> +++-
>  drivers/gpu/drm/i915/i915_reg.h|1 +
>  3 files changed, 80 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3ac71db..5e9ea36 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1055,6 +1055,8 @@ typedef struct drm_i915_private {
>  
>   struct i915_gpu_error gpu_error;
>  
> + struct drm_i915_gem_object *vlv_pctx;
> +
>   /* list of fbdev register on this device */
>   struct intel_fbdev *fbdev;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
> b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 67d3510..c06056a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -62,7 +62,10 @@ static unsigned long i915_stolen_to_physical(struct 
> drm_device *dev)
>* its value of TOLUD.
>*/
>   base = 0;
> - if (INTEL_INFO(dev)->gen >= 6) {
> + if (IS_VALLEYVIEW(dev)) {
> + pci_read_config_dword(pdev, 0x5c, &base);
> + base &= ((1<<19) - 1);
> + } if (INTEL_INFO(dev)->gen >= 6) {

else if

>   /* Read Base Data of Stolen Memory Register (BDSM) directly.
>* Note that there is also a MCHBAR miror at 0x1080c0 or
>* we could use device 2:0x5c instead.
> @@ -172,14 +175,82 @@ void i915_gem_stolen_cleanup_compression(struct 
> drm_device *dev)
>   dev_priv->cfb_size = 0;
>  }
>  
> +static void valleyview_setup_pctx(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_i915_gem_object *pctx;
> + unsigned long pctx_paddr;
> + u32 pcbr;
> + int pctx_size = 24*1024;
> +
> + pcbr = I915_READ(VLV_PCBR);
> + if (pcbr) {
> + /* BIOS set it up already, grab the pre-alloc'd space */
> + int pcbr_offset;
> +
> + pcbr_offset = (pcbr & (~4095)) - dev_priv->mm.stolen_base;
s/4095/PAGE_MASK?
> + pctx = 
> i915_gem_object_create_stolen_for_preallocated(dev_priv->dev,
> +   
> pcbr_offset,
> +   
> pcbr_offset,
> +   
> pctx_size);

Can you remind me how we can get away with the assumption that gtt
offset has a 1:1 mapping with the stolen offset?

> + /* We don't need to track it since we don't own it */
> + return;
> + }
> +
> + /*
> +  * From the Gunit register HAS:
> +  * The Gfx driver is expected to program this register and ensure
> +  * proper allocation within Gfx stolen memory.  For example, this
> +  * register should be programmed such than the PCBR range does not
> +  * overlap with other ranges, such as the frame buffer, protected
> +  * memory, or any other relevant ranges.
> +  */
> + pctx = i915_gem_object_create_stolen(dev, pctx_size);
> + if (!pctx) {
> + DRM_DEBUG("not enough stolen space for PCTX, disabling\n");
> + return;
> + }
> +
> + dev_priv->vlv_pctx = pctx;
> + pctx_paddr = dev_priv->mm.stolen_base + pctx->stolen->start;
> + I915_WRITE(VLV_PCBR, pctx_paddr);

Does the powerctx need to be reinitialized after GPU reset?

> +}

> +
> +static void valleyview_cleanup_pctx(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + if (!dev_priv->vlv_pctx)
> + return;
> +
> + i915_gem_object_release_stolen(dev_priv->vlv_pctx);
> + I915_WRITE(VLV_PCBR, 0);
> +}
> +
>  void i915_gem_cleanup_stolen(struct drm_device *dev)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
>  
>   i915_gem_stolen_cleanup_compression(dev);
> + if (IS_VALLEYVIEW(dev))
> + valleyview_cleanup_pctx(dev);
>   drm_mm_takedown(&dev_priv->mm.stolen);
>  }
>  
> +/* On VLV make sure we create pre-alloc'd objects for BIOS bits */
> +static void valleyview_init_bios_stolen(struct drm_i915_private *dev_priv)
> +{
> + struct drm_i915_gem_object *bios_stolen;
> + int bios_offset;
> +
> + /* Top 1M of stolen space is used by firmware */
> + bios_offset = dev_priv->gtt.stolen_size - 1024*1024;
> + bios_stolen = 
> i915_gem_object_create_stolen_for_preallocated(dev_priv->dev,
> +  
> bios_offs

Re: [Intel-gfx] [PATCH 2/4] drm/i915: go back to switch for VLV mem freq detection v2

2013-05-03 Thread Ben Widawsky
On Thu, May 02, 2013 at 10:48:08AM -0700, Jesse Barnes wrote:
> Both the docs and the existing code were wrong.  So fix both and use a
> switch statement like we do elsewhere to make things simple & clear.
> 
> Signed-off-by: Jesse Barnes 
> ---
>  drivers/gpu/drm/i915/intel_pm.c |   13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0f4b46e..556b989 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2902,7 +2902,18 @@ static void valleyview_enable_rps(struct drm_device 
> *dev)
>  GEN7_RC_CTL_TO_MODE);
>  
>   valleyview_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &val);
> - dev_priv->mem_freq = 800 + (266 * (val >> 6) & 3);
> + switch ((val >> 6) & 3) {
> + case 0:
> + case 1:
> + dev_priv->mem_freq = 800;
> + break;
> + case 2:
> + dev_priv->mem_freq = 1066;
> + break;
> + case 3:
> + dev_priv->mem_freq = 1333;
> + break;
> + }
>   DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
>  
>   DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & 0x10 ? "yes" : "no");

The code does what the author wants it to, but I don't have the doc that
says this:
Reviewed-by: Ben Widawsky 

Since I've set up a precedent of providing what I think are better
options:
freq_lut[] = {800, 800, 1066, 1333};
freq_lut[(val >> 6) & 3];

[really ugly one-liner redacted]

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


[Intel-gfx] [PATCH 2/2] drm/i915: Add references to some workaround we implement

2013-05-03 Thread Damien Lespiau
We did not mention the workaround name when implementing those. This
should help us track what we already implement.

Signed-off-by: Damien Lespiau 
---
 drivers/gpu/drm/i915/i915_gem_context.c | 1 +
 drivers/gpu/drm/i915/intel_ddi.c| 2 ++
 drivers/gpu/drm/i915/intel_display.c| 4 ++--
 drivers/gpu/drm/i915/intel_pm.c | 3 +++
 drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
 5 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 9e8c685..c42a411 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -325,6 +325,7 @@ mi_set_context(struct intel_ring_buffer *ring,
if (ret)
return ret;
 
+   /* WaProgramMiArbOnOffAroundMiSetContext:ivb,vlv,hsw */
if (IS_GEN7(ring->dev))
intel_ring_emit(ring, MI_ARB_ON_OFF | MI_ARB_DISABLE);
else
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 3ff4de6..83e2365 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -174,6 +174,8 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
 * mode set "sequence for CRT port" document:
 * - TP1 to TP2 time with the default value
 * - FDI delay to 90h
+*
+* WaFDIAutoLinkSetTimingOverrride:hsw
 */
I915_WRITE(_FDI_RXA_MISC, FDI_RX_PWRDN_LANE1_VAL(2) |
  FDI_RX_PWRDN_LANE0_VAL(2) |
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 6504337..7dbec41 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4115,8 +4115,8 @@ static int intel_crtc_compute_config(struct drm_crtc 
*crtc,
if (!pipe_config->timings_set)
drm_mode_set_crtcinfo(adjusted_mode, 0);
 
-   /* WaPruneModeWithIncorrectHsyncOffset: Cantiga+ cannot handle modes
-* with a hsync front porch of 0.
+   /* Cantiga+ cannot handle modes with a hsync front porch of 0.
+* WaPruneModeWithIncorrectHsyncOffset:ctg,elk,ilk,snb,ivb,vlv,hsw.
 */
if ((INTEL_INFO(dev)->gen > 4 || IS_G4X(dev)) &&
adjusted_mode->hsync_start == adjusted_mode->hdisplay)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5130d26..74b34d4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4559,6 +4559,7 @@ static void __gen6_gt_force_wake_get(struct 
drm_i915_private *dev_priv)
FORCEWAKE_ACK_TIMEOUT_MS))
DRM_ERROR("Timed out waiting for forcewake to ack request.\n");
 
+   /* WaRsForcewakeWaitTC0:snb */
__gen6_gt_wait_for_thread_c0(dev_priv);
 }
 
@@ -4590,6 +4591,7 @@ static void __gen6_gt_force_wake_mt_get(struct 
drm_i915_private *dev_priv)
FORCEWAKE_ACK_TIMEOUT_MS))
DRM_ERROR("Timed out waiting for forcewake to ack request.\n");
 
+   /* WaRsForcewakeWaitTC0:ivb,hsw */
__gen6_gt_wait_for_thread_c0(dev_priv);
 }
 
@@ -4693,6 +4695,7 @@ static void vlv_force_wake_get(struct drm_i915_private 
*dev_priv)
FORCEWAKE_ACK_TIMEOUT_MS))
DRM_ERROR("Timed out waiting for media to ack forcewake 
request.\n");
 
+   /* WaRsForcewakeWaitTC0:vlv */
__gen6_gt_wait_for_thread_c0(dev_priv);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 1d5d613..3d2c236 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -515,6 +515,8 @@ static int init_render_ring(struct intel_ring_buffer *ring)
/* We need to disable the AsyncFlip performance optimisations in order
 * to use MI_WAIT_FOR_EVENT within the CS. It should already be
 * programmed to '1' on all products.
+*
+* WaDisableAsyncFlipPerfMode:snb,ivb,hsw,vlv
 */
if (INTEL_INFO(dev)->gen >= 6)
I915_WRITE(MI_MODE, 
_MASKED_BIT_ENABLE(ASYNC_FLIP_PERF_DISABLE));
-- 
1.8.1.4

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


[Intel-gfx] [PATCH 1/2] drm/i915: Add platform information to implemented workarounds

2013-05-03 Thread Damien Lespiau
Signed-off-by: Damien Lespiau 
---
 drivers/gpu/drm/i915/i915_drv.c |  6 ++--
 drivers/gpu/drm/i915/intel_pm.c | 77 +
 2 files changed, 42 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 624cdfc..40b5787 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1198,9 +1198,9 @@ MODULE_LICENSE("GPL and additional rights");
 static void
 ilk_dummy_write(struct drm_i915_private *dev_priv)
 {
-   /* WaIssueDummyWriteToWakeupFromRC6: Issue a dummy write to wake up the
-* chip from rc6 before touching it for real. MI_MODE is masked, hence
-* harmless to write 0 into. */
+   /* WaIssueDummyWriteToWakeupFromRC6:ilk Issue a dummy write to wake up
+* the chip from rc6 before touching it for real. MI_MODE is masked,
+* hence harmless to write 0 into. */
I915_WRITE_NOTRACE(MI_MODE, 0);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0f4b46e..5130d26 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3797,7 +3797,7 @@ static void ironlake_init_clock_gating(struct drm_device 
*dev)
   _3D_CHICKEN2_WM_READ_PIPELINED << 16 |
   _3D_CHICKEN2_WM_READ_PIPELINED);
 
-   /* WaDisableRenderCachePipelinedFlush */
+   /* WaDisableRenderCachePipelinedFlush:ilk */
I915_WRITE(CACHE_MODE_0,
   _MASKED_BIT_ENABLE(CM0_PIPELINED_RENDER_FLUSH_DISABLE));
 
@@ -3864,11 +3864,11 @@ static void gen6_init_clock_gating(struct drm_device 
*dev)
   I915_READ(ILK_DISPLAY_CHICKEN2) |
   ILK_ELPIN_409_SELECT);
 
-   /* WaDisableHiZPlanesWhenMSAAEnabled */
+   /* WaDisableHiZPlanesWhenMSAAEnabled:snb */
I915_WRITE(_3D_CHICKEN,
   
_MASKED_BIT_ENABLE(_3D_CHICKEN_HIZ_PLANE_DISABLE_MSAA_4X_SNB));
 
-   /* WaSetupGtModeTdRowDispatch */
+   /* WaSetupGtModeTdRowDispatch:snb */
if (IS_SNB_GT1(dev))
I915_WRITE(GEN6_GT_MODE,
   
_MASKED_BIT_ENABLE(GEN6_TD_FOUR_ROW_DISPATCH_DISABLE));
@@ -3895,8 +3895,8 @@ static void gen6_init_clock_gating(struct drm_device *dev)
 * According to the spec, bit 11 (RCCUNIT) must also be set,
 * but we didn't debug actual testcases to find it out.
 *
-* Also apply WaDisableVDSUnitClockGating and
-* WaDisableRCPBUnitClockGating.
+* Also apply WaDisableVDSUnitClockGating:snb and
+* WaDisableRCPBUnitClockGating:snb.
 */
I915_WRITE(GEN6_UCGCTL2,
   GEN7_VDSUNIT_CLOCK_GATE_DISABLE |
@@ -3927,7 +3927,7 @@ static void gen6_init_clock_gating(struct drm_device *dev)
   ILK_DPARBUNIT_CLOCK_GATE_ENABLE  |
   ILK_DPFDUNIT_CLOCK_GATE_ENABLE);
 
-   /* WaMbcDriverBootEnable */
+   /* WaMbcDriverBootEnable:snb */
I915_WRITE(GEN6_MBCTL, I915_READ(GEN6_MBCTL) |
   GEN6_MBCTL_ENABLE_BOOT_FETCH);
 
@@ -3957,7 +3957,6 @@ static void gen7_setup_fixed_func_scheduler(struct 
drm_i915_private *dev_priv)
reg |= GEN7_FF_VS_SCHED_HW;
reg |= GEN7_FF_DS_SCHED_HW;
 
-   /* WaVSRefCountFullforceMissDisable */
if (IS_HASWELL(dev_priv->dev))
reg &= ~GEN7_FF_VS_REF_CNT_FFME;
 
@@ -3988,21 +3987,21 @@ static void haswell_init_clock_gating(struct drm_device 
*dev)
I915_WRITE(WM1_LP_ILK, 0);
 
/* According to the spec, bit 13 (RCZUNIT) must be set on IVB.
-* This implements the WaDisableRCZUnitClockGating workaround.
+* This implements the WaDisableRCZUnitClockGating:hsw workaround.
 */
I915_WRITE(GEN6_UCGCTL2, GEN6_RCZUNIT_CLOCK_GATE_DISABLE);
 
-   /* Apply the WaDisableRHWOOptimizationForRenderHang workaround. */
+   /* Apply the WaDisableRHWOOptimizationForRenderHang:hsw workaround. */
I915_WRITE(GEN7_COMMON_SLICE_CHICKEN1,
   GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC);
 
-   /* WaApplyL3ControlAndL3ChickenMode requires those two on Ivy Bridge */
+   /* WaApplyL3ControlAndL3ChickenMode:hsw */
I915_WRITE(GEN7_L3CNTLREG1,
GEN7_WA_FOR_GEN7_L3_CONTROL);
I915_WRITE(GEN7_L3_CHICKEN_MODE_REGISTER,
GEN7_WA_L3_CHICKEN_MODE);
 
-   /* This is required by WaCatErrorRejectionIssue */
+   /* This is required by WaCatErrorRejectionIssue:hsw */
I915_WRITE(GEN7_SQ_CHICKEN_MBCUNIT_CONFIG,
I915_READ(GEN7_SQ_CHICKEN_MBCUNIT_CONFIG) |
GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB);
@@ -4014,17 +4013,18 @@ static void haswell_init_clock_gating(struct drm_device 
*dev)
intel_flush_display_plane(dev_priv, pipe);
}
 
+   /* WaVSRefCountFullforceMissDisable:hsw */
gen7_setup_fixed_func_scheduler(dev_priv);
 
-   /* WaDis

[Intel-gfx] [PATCH i-g-t] scripts: Add a script to list implemented workarounds

2013-05-03 Thread Damien Lespiau
We document the implemented workarounds with

  workaround_name:platforms

with platforms being a comma separated list of 3-letters platform names.

This scripts gather those tags and output a summary of implemented work
arounds. Example usages:

$ ./scripts/list-workarounds ~/gfx/sources/linux-2.6/
WaApplyL3ControlAndL3ChickenMode: hsw, ivb, vlv
WaCatErrorRejectionIssue: hsw, ivb, vlv
WaDisable4x2SubspanOptimization: hsw, ivb
WaDisableBackToBackFlipFix: ivb, vlv
WaDisableDopClockGating: vlv


$ ./scripts/list-workarounds ~/gfx/sources/linux-2.6/ -p ivb
WaApplyL3ControlAndL3ChickenMode
WaCatErrorRejectionIssue
WaDisable4x2SubspanOptimization
WaDisableBackToBackFlipFix
WaDisableEarlyCull
...

Signed-off-by: Damien Lespiau 
---
 scripts/list-workarounds | 107 +++
 1 file changed, 107 insertions(+)
 create mode 100755 scripts/list-workarounds

diff --git a/scripts/list-workarounds b/scripts/list-workarounds
new file mode 100755
index 000..7bfd82d
--- /dev/null
+++ b/scripts/list-workarounds
@@ -0,0 +1,107 @@
+#!/usr/bin/env python
+
+import os,sys
+import optparse
+import subprocess
+import re
+import operator
+
+# map of Workaround names -> (list of platforms)
+workarounds = {}
+verbose = False
+
+def find_nth(haystack, needle, n):
+   start = haystack.find(needle)
+   while start >= 0 and n > 1:
+   start = haystack.find(needle, start + len(needle))
+   n -= 1
+   return start
+
+valid_platforms = ('ctg', 'elk', 'ilk', 'snb', 'ivb', 'vlv', 'hsw')
+def parse_platforms(p):
+   l =  p.split(',')
+   for p in l:
+   if p not in valid_platforms:
+   sys.stdout.write("unknown platform %s\n" % p)
+   return l
+
+wa_re = re.compile('(?PWa[A-Z0-9][a-zA-Z0-9_]+):(?P[a-z,]+)')
+waname_re = re.compile('(?PWa[A-Z0-9][a-zA-Z0-9_]+)')
+def parse(me):
+   for line in me.splitlines():
+   match = wa_re.search(line)
+   if not match:
+   if not verbose:
+   continue
+
+   # Those lines come from a git grep that looks for Wa
+   # names, so if we don't match wa_re here it's because
+   # no platform has been specified
+   name = waname_re.search(line).group('name')
+   path = line[:find_nth(line, ':', 2)]
+   sys.stdout.write("%s: no platform for %s\n"
+% (path, name))
+   continue
+
+   wa_name = match.group('name')
+   platforms = match.group('platforms')
+
+   if wa_name in workarounds:
+   workarounds[wa_name] += parse_platforms(platforms)
+   else:
+   workarounds[wa_name] = parse_platforms(platforms)
+
+
+def execute(cmd):
+   p = subprocess.Popen(cmd, stdout=subprocess.PIPE,
+stderr=subprocess.PIPE)
+   out, err = p.communicate()
+   return out, err
+
+def parse_options(args):
+   usage = "Usage: list-workarounds [options] path-to-kernel"
+   parser = optparse.OptionParser(usage, version=1.0)
+
+   parser.add_option("-v", "--verbose", action="store_true",
+ dest="verbose", default=False,
+ help="be more verbose")
+
+   parser.add_option("-p", "--platform", dest="platform", default=None,
+ help="List workarounds for the specified platform")
+
+   (options, args) = parser.parse_args()
+
+   return (options, args)
+
+if __name__ == '__main__':
+   (options, args) = parse_options(sys.argv[1:])
+   verbose = options.verbose
+
+   if not len(args):
+   sys.stderr.write("error: A path to a kernel tree is required\n")
+   sys.exit(1)
+
+   kernel_path = args[0]
+   kconfig = os.path.join(kernel_path, 'Kconfig')
+   if not os.path.isfile(kconfig):
+   sys.stderr.write("error: %s does not point to a kernel tree \n"
+% kernel_path)
+   sys.exit(1)
+
+   i915_dir = os.path.join(kernel_path, 'drivers', 'gpu', 'drm', 'i915')
+   olddir = os.getcwd()
+   os.chdir(kernel_path)
+   work_arounds, err = execute(['git', 'grep', '-n',
+'-e', 'Wa[A-Z0-9][a-zA-Z0-9_]\+',
+i915_dir])
+   os.chdir(olddir)
+   if err:
+   print(err)
+   sys.exit(1)
+
+   parse(work_arounds)
+   for wa in sorted(workarounds.iterkeys()):
+   if not options.platform:
+   print("%s: %s" % (wa, ', '.join(workarounds[wa])))
+   elif options.platform in workarounds[wa]:
+   print(wa)
-- 
1.8.1.4

___
Intel-gfx mailing list
Intel-gfx@l

[Intel-gfx] [PATCH V2] drm/i915: force full modeset if the connector is in DPMS OFF mode

2013-05-03 Thread Egbert Eich
From: Imre Deak 

Currently the driver's assumed behavior for a modeset with an attached
FB is that the corresponding connector will be switched to DPMS ON mode
if it happened to be in DPMS OFF (or another power save mode). This
wasn't enforced though if only the FB changed, everything else (format,
connector etc.) remaining the same. In this case we only set the new FB
base and left the connector in the old power save mode.

Fix this by forcing a full modeset whenever there is an attached FB and
any affected connector is in a power save mode.

V_2: Run the test for encoders in power save mode outside the the
test for fb change: user space may have just disabled the encoders
but left everything else in place. Make sure the connector list is
not empty before running this test.

Signed-off-by: Imre Deak 
Signed-off-by: Egbert Eich 
---
 drivers/gpu/drm/i915/intel_display.c | 24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 2939524..992d469 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8380,6 +8380,21 @@ static void intel_set_config_restore_state(struct 
drm_device *dev,
}
 }
 
+static bool
+connector_off(struct drm_crtc *crtc, struct drm_connector *connectors,
+ int num_connectors)
+{
+   int i;
+
+   for (i = 0; i < num_connectors; i++)
+   if (connectors[i].encoder &&
+   connectors[i].encoder->crtc == crtc &&
+   connectors[i].dpms != DRM_MODE_DPMS_ON)
+   return true;
+
+   return false;
+}
+
 static void
 intel_set_config_compute_mode_changes(struct drm_mode_set *set,
  struct intel_set_config *config)
@@ -8387,7 +8402,11 @@ intel_set_config_compute_mode_changes(struct 
drm_mode_set *set,
 
/* We should be able to check here if the fb has the same properties
 * and then just flip_or_move it */
-   if (set->crtc->fb != set->fb) {
+   if (set->connectors != NULL && 
+   connector_off(set->crtc, *set->connectors,
+ set->num_connectors)) {
+   config->mode_changed = true;
+   } else if (set->crtc->fb != set->fb) {
/* If we have no fb then treat it as a full mode set */
if (set->crtc->fb == NULL) {
DRM_DEBUG_KMS("crtc has no fb, full mode set\n");
@@ -8397,8 +8416,9 @@ intel_set_config_compute_mode_changes(struct drm_mode_set 
*set,
} else if (set->fb->pixel_format !=
   set->crtc->fb->pixel_format) {
config->mode_changed = true;
-   } else
+   } else {
config->fb_changed = true;
+   }
}
 
if (set->fb && (set->x != set->crtc->x || set->y != set->crtc->y))
-- 
1.8.1.4

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


Re: [Intel-gfx] [PATCH] drm/i915: force full modeset if the connector is in DPMS OFF mode

2013-05-03 Thread Egbert Eich
On Fri, May 03, 2013 at 02:22:09PM +0300, Imre Deak wrote:
> Currently the driver's assumed behavior for a modeset with an attached
> FB is that the corresponding connector will be switched to DPMS ON mode
> if it happened to be in DPMS OFF (or another power save mode). This
> wasn't enforced though if only the FB changed, everything else (format,
> connector etc.) remaining the same. In this case we only set the new FB
> base and left the connector in the old power save mode.
> 
> Fix this by forcing a full modeset whenever there is an attached FB and
> any affected connector is in a power save mode.
> 
> Signed-off-by: Imre Deak 

[..]

> @@ -8397,8 +8412,12 @@ intel_set_config_compute_mode_changes(struct 
> drm_mode_set *set,
>   } else if (set->fb->pixel_format !=
>  set->crtc->fb->pixel_format) {
>   config->mode_changed = true;
> - } else
> + } else if (crtc_connector_off(set->crtc, *set->connectors,
> +   set->num_connectors)) {
> + config->mode_changed = true;
> + } else {
>   config->fb_changed = true;
> + }
>   }
>  
>   if (set->fb && (set->x != set->crtc->x || set->y != set->crtc->y))

On https://bugs.freedesktop.org/show_bug.cgi?id=64178
I had a similar problem for which I found a very similar 'workaround':

Assuming the Xserver is displaying a mode a on output A, doing a:

xrandr --output A --off
xrandr --output A --mode a

will not light up the screen.

This all sounds quite similar, to the issues describe by this patch,
however the patch as is will not fix this issue: With this setup the
fb does not change. The crtc_connector_off() test however only runs if
set->crtc->fb != set->fb is true.

Thus the connector_off() test needs to happen before:

-   if (set->crtc->fb != set->fb) {
+   if (set->connectors != NULL && 
+   connector_off(set->crtc, *set->connectors,
+ set->num_connectors)) {
+   config->mode_changed = true;
+   } else if (set->crtc->fb != set->fb) {

Mind the test for set->connectors != NULL as now the connectors list
may be empty.

Cheers,
Egbert.

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


[Intel-gfx] [PATCH] drm/i915: Retrieve the current mode upon KMS takeover v2

2013-05-03 Thread Jesse Barnes
Read the current hardware state to retrieve the active mode and populate
our CRTC config if that mode matches our presumptions.

v2: check that get_hw_state gave us a valid pipe (Imre)
add clock_get for ILK+ (Jesse)

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/intel_crt.c |   22 +++
 drivers/gpu/drm/i915/intel_display.c |  306 --
 drivers/gpu/drm/i915/intel_dp.c  |   22 +++
 drivers/gpu/drm/i915/intel_drv.h |7 +-
 drivers/gpu/drm/i915/intel_dvo.c |   36 ++--
 drivers/gpu/drm/i915/intel_hdmi.c|   22 +++
 drivers/gpu/drm/i915/intel_lvds.c|   27 ++-
 drivers/gpu/drm/i915/intel_sdvo.c|   23 +++
 8 files changed, 328 insertions(+), 137 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 991e530..0974d31 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -81,6 +81,27 @@ static bool intel_crt_get_hw_state(struct intel_encoder 
*encoder,
return true;
 }
 
+static unsigned intel_crt_get_mode_flags(struct intel_encoder *encoder)
+{
+   struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+   struct intel_crt *crt = intel_encoder_to_crt(encoder);
+   u32 tmp, flags = 0;
+
+   tmp = I915_READ(crt->adpa_reg);
+
+   if (tmp & ADPA_HSYNC_ACTIVE_HIGH)
+   flags |= DRM_MODE_FLAG_PHSYNC;
+   else
+   flags |= DRM_MODE_FLAG_NHSYNC;
+
+   if (tmp & ADPA_VSYNC_ACTIVE_HIGH)
+   flags |= DRM_MODE_FLAG_PVSYNC;
+   else
+   flags |= DRM_MODE_FLAG_NVSYNC;
+
+   return flags;
+}
+
 static void intel_disable_crt(struct intel_encoder *encoder)
 {
struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
@@ -784,6 +805,7 @@ void intel_crt_init(struct drm_device *dev)
crt->base.compute_config = intel_crt_compute_config;
crt->base.disable = intel_disable_crt;
crt->base.enable = intel_enable_crt;
+   crt->base.get_mode_flags = intel_crt_get_mode_flags;
if (I915_HAS_HOTPLUG(dev))
crt->base.hpd_pin = HPD_CRT;
if (HAS_DDI(dev))
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 625a8cd..6316dca 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4969,6 +4969,122 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
return ret;
 }
 
+/* Returns the clock of the currently programmed mode of the given pipe. */
+static int i9xx_crtc_clock_get(struct drm_crtc *crtc)
+{
+   struct drm_device *dev = crtc->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+   enum pipe pipe = intel_crtc->pipe;
+   u32 dpll = I915_READ(DPLL(pipe));
+   u32 fp;
+   intel_clock_t clock;
+
+   if ((dpll & DISPLAY_RATE_SELECT_FPA1) == 0)
+   fp = I915_READ(FP0(pipe));
+   else
+   fp = I915_READ(FP1(pipe));
+
+   clock.m1 = (fp & FP_M1_DIV_MASK) >> FP_M1_DIV_SHIFT;
+   if (IS_PINEVIEW(dev)) {
+   clock.n = ffs((fp & FP_N_PINEVIEW_DIV_MASK) >> FP_N_DIV_SHIFT) 
- 1;
+   clock.m2 = (fp & FP_M2_PINEVIEW_DIV_MASK) >> FP_M2_DIV_SHIFT;
+   } else {
+   clock.n = (fp & FP_N_DIV_MASK) >> FP_N_DIV_SHIFT;
+   clock.m2 = (fp & FP_M2_DIV_MASK) >> FP_M2_DIV_SHIFT;
+   }
+
+   if (!IS_GEN2(dev)) {
+   if (IS_PINEVIEW(dev))
+   clock.p1 = ffs((dpll & 
DPLL_FPA01_P1_POST_DIV_MASK_PINEVIEW) >>
+   DPLL_FPA01_P1_POST_DIV_SHIFT_PINEVIEW);
+   else
+   clock.p1 = ffs((dpll & DPLL_FPA01_P1_POST_DIV_MASK) >>
+  DPLL_FPA01_P1_POST_DIV_SHIFT);
+
+   switch (dpll & DPLL_MODE_MASK) {
+   case DPLLB_MODE_DAC_SERIAL:
+   clock.p2 = dpll & DPLL_DAC_SERIAL_P2_CLOCK_DIV_5 ?
+   5 : 10;
+   break;
+   case DPLLB_MODE_LVDS:
+   clock.p2 = dpll & DPLLB_LVDS_P2_CLOCK_DIV_7 ?
+   7 : 14;
+   break;
+   default:
+   DRM_DEBUG_KMS("Unknown DPLL mode %08x in programmed "
+ "mode\n", (int)(dpll & DPLL_MODE_MASK));
+   return 0;
+   }
+
+   /* XXX: Handle the 100Mhz refclk */
+   intel_clock(dev, 96000, &clock);
+   } else {
+   bool is_lvds = (pipe == 1) && (I915_READ(LVDS) & LVDS_PORT_EN);
+
+   if (is_lvds) {
+   clock.p1 = ffs((dpll & 
DPLL_FPA01_P1_POST_DIV_MASK_I830_LVDS) >>
+  DPLL_FPA01_P1_POST_DIV_SHIFT);
+   clock.p2 = 14;
+
+   if ((dpll & 

Re: [Intel-gfx] [git pull] drm for 3.10-rc1

2013-05-03 Thread Josh Boyer
On Fri, May 03, 2013 at 06:08:52PM +0200, Daniel Vetter wrote:
>On Fri, May 3, 2013 at 4:39 PM, Josh Boyer  wrote:
>> On Fri, May 03, 2013 at 02:25:57AM +0100, Dave Airlie wrote:
>>>The following changes since commit b6a9b7f6b1f21735a7456d534dc0e68e61359d2c:
>>>
>>>  mm: prevent mmap_cache race in find_vma() (2013-04-04 11:46:28 -0700)
>>>
>>>are available in the git repository at:
>>>
>>>  git://people.freedesktop.org/~airlied/linux.git drm-next
>>>
>>>for you to fetch changes up to 307b9c022720f9de90d58e51743e01e9a42aec59:
>>>
>>>  qxl: update to new idr interfaces. (2013-05-03 10:37:20 +1000)
>>>
>>>
>>
>> So something in this batch of commits breaks a Macbook Pro Retina I have
>> sitting here.  If I boot a Fedora kernel based on  Linux
>> v3.9-8153-g5a148af, things boot up as they did previously with 3.9.0 and
>> are generally working fine.  If I boot with a kernel based on Linux
>> v3.9-8933-gce85722, it will boot but as soon as plymouth starts, I get
>> nothing but static on the screen (like 1950s TV static).
>>
>> This machine is using i915 graphics, so I booted with nomodeset and did
>> the 'modprobe drm debug=15; modprobe i915 modeset=1' trick and it
>> repeated the failure with that.  I've gathered a bunch of data like
>> dmesg, an intel_reg_snapshot, etc.  It's all attached below.
>>
>> I'll start bisecting to see if I can narrow down the commit that broke
>> things, but I thought I would give a heads up now in case anyone has any
>> ideas.
>
>Looked through the log files and didn't see a clue. And usually DP
>tends to fail pretty noisily. So I think the bisect result would be
>interestin. Meanwhile:

Yeah, working on that.

>- do you have drm.debug=0xe dmesg for a working kernel, too?

No, but I can try and get one.

>- is the panel completely dead or is just the backlight on (or panel
>on but backlight off)?

Backlight is on.  Literally static flickering on the screen.
Occasionally it will flicker from static to black back and forth.  But
the backlight is clearly on and what is trying to render (the GDM screen
normally) is just coming up as static.

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


Re: [Intel-gfx] [PATCH] drm/i915: Propagate errors back from fb set-base

2013-05-03 Thread Daniel Vetter
On Fri, May 3, 2013 at 6:36 PM, Chris Wilson  wrote:
> Along the modesetting short cut where we skip trying to do a full
> modeset and instead simply update the framebuffer base registers, we
> failed to handle any errors reported.
>
> Signed-off-by: Chris Wilson 


This regression has been introduced in

commit 94352cf9a5328bb1a44288e6c2c1276695f8a356
Author: Daniel Vetter 
Date:   Thu Jul 5 22:51:56 2012 +0200

drm/i915: push crtc->fb update into pipe_set_base

/me hangs head in shame
--
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: force full modeset if the connector is in DPMS OFF mode

2013-05-03 Thread Daniel Vetter
On Fri, May 3, 2013 at 6:28 PM, Chris Wilson  wrote:
> Not quite. It was added afterwards, I know for it is one of my mistakes
> that I belatedly recognised as trying to workaround a bug in UXA. And we
> then layered on further bugs to try and patch up the glaring holes then
> introduced.

I've done some digging around and found some cool stuff, but no change
which introduce the original bug that we've never consulted dpms state
in drm_crtc_helper_set_mode but just blindly enabled encoders. Most of
the commits I've dug out try to paper over that mismatch by updating
dpms state, too. E.g. bf9dc102 which was then fixed up a bit in
811aaa55 but I don't see any mention of someone explicitly kill dpms
checks.

Of course for DP support we've started to add dpms state checks to our
dp encoder (since double-enable kills DP). Then duplicated the
drm_connector->dpms into our own intel_dp->dpms state since the crtc
helpers liked to frob around with dpms in a rather ill-defined way.
But that's just hilarity and flailing around on top, nothing that I
could see fundamentally changed that a full setcrtc implicitly enabled
the encoders/crtc hw. And since we have our own modeset code that all
disappeared (since a full modeset simply force-updates the dpms state
to reflect reality).

But nowhere have a found a commit to support your claim, just lots of
flailing around due to the ill-defined semantics of this all. Care to
do some more digging?

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


[Intel-gfx] [PATCH] drm/i915: Propagate errors back from fb set-base

2013-05-03 Thread Chris Wilson
Along the modesetting short cut where we skip trying to do a full
modeset and instead simply update the framebuffer base registers, we
failed to handle any errors reported.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/intel_display.c |   25 ++---
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 2939524..62eafc7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8572,11 +8572,6 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
 
ret = intel_set_mode(set->crtc, set->mode,
 set->x, set->y, set->fb);
-   if (ret) {
-   DRM_ERROR("failed to set mode on [CRTC:%d], err = %d\n",
- set->crtc->base.id, ret);
-   goto fail;
-   }
} else if (config->fb_changed) {
intel_crtc_wait_for_pending_flips(set->crtc);
 
@@ -8584,18 +8579,18 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
  set->x, set->y, set->fb);
}
 
-   intel_set_config_free(config);
-
-   return 0;
-
+   if (ret) {
+   DRM_ERROR("failed to set mode on [CRTC:%d], err = %d\n",
+ set->crtc->base.id, ret);
 fail:
-   intel_set_config_restore_state(dev, config);
+   intel_set_config_restore_state(dev, config);
 
-   /* Try to restore the config */
-   if (config->mode_changed &&
-   intel_set_mode(save_set.crtc, save_set.mode,
-  save_set.x, save_set.y, save_set.fb))
-   DRM_ERROR("failed to restore config after modeset failure\n");
+   /* Try to restore the config */
+   if (config->mode_changed &&
+   intel_set_mode(save_set.crtc, save_set.mode,
+  save_set.x, save_set.y, save_set.fb))
+   DRM_ERROR("failed to restore config after modeset 
failure\n");
+   }
 
 out_config:
intel_set_config_free(config);
-- 
1.7.10.4

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


Re: [Intel-gfx] [PATCH] drm/i915: force full modeset if the connector is in DPMS OFF mode

2013-05-03 Thread Chris Wilson
On Fri, May 03, 2013 at 05:55:31PM +0200, Daniel Vetter wrote:
> On Fri, May 3, 2013 at 2:22 PM, Chris Wilson  wrote:
> > On Fri, May 03, 2013 at 02:55:45PM +0300, Imre Deak wrote:
> >> On Fri, 2013-05-03 at 12:44 +0100, Chris Wilson wrote:
> >> > On Fri, May 03, 2013 at 02:22:09PM +0300, Imre Deak wrote:
> >> > > Currently the driver's assumed behavior for a modeset with an attached
> >> > > FB is that the corresponding connector will be switched to DPMS ON mode
> >> > > if it happened to be in DPMS OFF (or another power save mode). This
> >> > > wasn't enforced though if only the FB changed, everything else (format,
> >> > > connector etc.) remaining the same. In this case we only set the new FB
> >> > > base and left the connector in the old power save mode.
> >> > >
> >> > > Fix this by forcing a full modeset whenever there is an attached FB and
> >> > > any affected connector is in a power save mode.
> >> > >
> >> > > Signed-off-by: Imre Deak 
> >> >
> >> > NAK. Papering over bugs, much?
> >>
> >> Ok. I posted this based on our IRC discussion where we seemed to agree
> >> that DPMS_ON is assumed already; just that it's not done consistently.
> >> That is if user space does a modeset now that ends up in a full modeset
> >> (lets say a new FB with different format) then the kernel will do a
> >> DPMS_ON. But if the new FB happens to be the same format then it won't.
> >> I thought this is inconsistent and should be fixed.
> >>
> >> Another way to make it consistent would be then to remove DPMS ON from
> >> the full modeset path..
> >
> > Right, we have mentioned in the past that the conflation between DPMS
> > and modesetting is a more recent bug that is going to cause us even more
> > trouble later. I am not sure how we can fix that as UXA already makes
> > many bad assumptions.
> 
> Afaict the conflagration of dpms on after a modeset has been even in
> the old crtc helpers. The only difference is that now we explicitly
> update the dpms state tracking to avoid double-encoder enables. That
> part allowed us to rip out tons of state tracking from tons of places.
> 
> So ever since kms happened, we have inconsistent setcrtc semantics.
> Stopping to force dpms on will horribly break existing userspace, so
> we can't do this. Making sure that the semantics are more consistent
> (especially if we further optimize modeset transitions for fastboot)
> seems like the lesser of two evils.

Not quite. It was added afterwards, I know for it is one of my mistakes
that I belatedly recognised as trying to workaround a bug in UXA. And we
then layered on further bugs to try and patch up the glaring holes then
introduced.

> We'll get a 2nd chance to get this right the atomic modeset ioctl.
> Until then, I want this duct-taped together, and Imre's patch looks
> like a viable approach. Working beats ugly semantics here imo.

I think the patch is quite ugly as we do not attempt to avoid the
unnecessary modeset step and so penalise working userspace code.  Now,
if you were first to make intel_modeset_affected_pipes() smarter, such
that the explicit fb set-base optimisation was no longer ever necessary,
things would be more interesting.
-Chris

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


Re: [Intel-gfx] [PATCH] drm/i915: force full modeset if the connector is in DPMS OFF mode

2013-05-03 Thread Jesse Barnes
On Fri, 3 May 2013 17:55:31 +0200
Daniel Vetter  wrote:

> On Fri, May 3, 2013 at 2:22 PM, Chris Wilson  wrote:
> > On Fri, May 03, 2013 at 02:55:45PM +0300, Imre Deak wrote:
> >> On Fri, 2013-05-03 at 12:44 +0100, Chris Wilson wrote:
> >> > On Fri, May 03, 2013 at 02:22:09PM +0300, Imre Deak wrote:
> >> > > Currently the driver's assumed behavior for a modeset with an attached
> >> > > FB is that the corresponding connector will be switched to DPMS ON mode
> >> > > if it happened to be in DPMS OFF (or another power save mode). This
> >> > > wasn't enforced though if only the FB changed, everything else (format,
> >> > > connector etc.) remaining the same. In this case we only set the new FB
> >> > > base and left the connector in the old power save mode.
> >> > >
> >> > > Fix this by forcing a full modeset whenever there is an attached FB and
> >> > > any affected connector is in a power save mode.
> >> > >
> >> > > Signed-off-by: Imre Deak 
> >> >
> >> > NAK. Papering over bugs, much?
> >>
> >> Ok. I posted this based on our IRC discussion where we seemed to agree
> >> that DPMS_ON is assumed already; just that it's not done consistently.
> >> That is if user space does a modeset now that ends up in a full modeset
> >> (lets say a new FB with different format) then the kernel will do a
> >> DPMS_ON. But if the new FB happens to be the same format then it won't.
> >> I thought this is inconsistent and should be fixed.
> >>
> >> Another way to make it consistent would be then to remove DPMS ON from
> >> the full modeset path..
> >
> > Right, we have mentioned in the past that the conflation between DPMS
> > and modesetting is a more recent bug that is going to cause us even more
> > trouble later. I am not sure how we can fix that as UXA already makes
> > many bad assumptions.
> 
> Afaict the conflagration of dpms on after a modeset has been even in
> the old crtc helpers. The only difference is that now we explicitly
> update the dpms state tracking to avoid double-encoder enables. That
> part allowed us to rip out tons of state tracking from tons of places.
> 
> So ever since kms happened, we have inconsistent setcrtc semantics.
> Stopping to force dpms on will horribly break existing userspace, so
> we can't do this. Making sure that the semantics are more consistent
> (especially if we further optimize modeset transitions for fastboot)
> seems like the lesser of two evils.
> 
> We'll get a 2nd chance to get this right the atomic modeset ioctl.
> Until then, I want this duct-taped together, and Imre's patch looks
> like a viable approach. Working beats ugly semantics here imo.

Agreed; there's definitely redundancy here, but turning the display on
at mode set time when it was clearly intended and implied by other
usages seems like following the policy of least surprise.

-- 
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 v2] drm/i915: unreference default context on module unload

2013-05-03 Thread Daniel Vetter
On Fri, May 03, 2013 at 08:53:20AM -0700, Ben Widawsky wrote:
> On Fri, May 03, 2013 at 04:29:08PM +0300, Mika Kuoppala wrote:
> > Before module unload is called, gpu_idle() will switch
> > to default context. This will increment ref count of base
> > object as the default context is 'running' on module unload
> > time. Unreference the drm object so that when context
> > is freed, base object is freed as well.
> > 
> > v2: added comment to explain the refcounts (Ben Widawsky)
> > 
> > Signed-off-by: Mika Kuoppala 
> 
> I left an r-b on the other patch. You should feel free to add it if you
> actually do what I say.
> 
> Reviewed-by: Ben Widawsky 

Merged, 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 Haswell pfit power well check v2

2013-05-03 Thread Daniel Vetter
On Fri, May 03, 2013 at 12:01:49PM -0300, Paulo Zanoni wrote:
> 2013/5/3 Mika Kuoppala :
> > Jesse Barnes  writes:
> >
> >> We can't read the pfit regs if the power well is off, so use the cached
> >> value.
> >>
> >> v2: re-add lost comment (Jesse)
> >> make sure the crtc using the fitter is actually enabled (Jesse)
> >>
> >> Signed-off-by: Jesse Barnes 
> >> ---
> >>  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 6504337..6be34f2 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -5918,7 +5918,7 @@ static void haswell_modeset_global_resources(struct 
> >> drm_device *dev)
> >>* sequence that's not yet available. Just in case desktop 
> >> eDP
> >>* on PORT D is possible on haswell, too. */
> >>   /* Even the eDP panel fitter is outside the always-on well. 
> >> */
> >> - if (I915_READ(PF_WIN_SZ(crtc->pipe)))
> >> + if (crtc->config.pch_pfit.size && crtc->base.enabled)
> >>   enable = true;
> >>   }
> >>
> >
> > Remove the now useless *dev_priv to remove compiler warning and then add
> >
> > Reviewed-by: Mika Kuoppala 
> 
> Yay, dmesg is clean again with this patch + Daniel's patch 06 + my
> local patches which I'll resend today.
> 
> With the warn pointed by Mika removed:
> Reviewed-by: Paulo Zanoni 
> Tested-by: Paulo Zanoni 

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


[Intel-gfx] [PATCH] [TRIVIAL] Fix declaration of intel_gmbus_{is_forced_bit/is_port_falid} in i915 driver.

2013-05-03 Thread dl9pf
From: Jan-Simon Möller 

Description:
intel_gmbus_is_forced_bit is no extern as its body is right below.
Likewise for intel_gmbus_is_port_valid.

This fixes a compilation issue with clang. An initial version of this patch
was developed by PaX Team .
This is respin of this patch.

Signed-off-by: Jan-Simon Möller 
CC: pagee...@freemail.hu
CC: daniel.vet...@ffwll.ch
CC: airl...@linux.ie
CC: intel-gfx@lists.freedesktop.org
CC: dri-de...@lists.freedesktop.org
CC: linux-ker...@vger.kernel.org
---
 drivers/gpu/drm/i915/i915_drv.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3ac71db..3c9ebc1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1806,7 +1806,7 @@ void i915_teardown_sysfs(struct drm_device *dev_priv);
 /* intel_i2c.c */
 extern int intel_setup_gmbus(struct drm_device *dev);
 extern void intel_teardown_gmbus(struct drm_device *dev);
-extern inline bool intel_gmbus_is_port_valid(unsigned port)
+static bool intel_gmbus_is_port_valid(unsigned port)
 {
return (port >= GMBUS_PORT_SSC && port <= GMBUS_PORT_DPD);
 }
@@ -1815,7 +1815,7 @@ extern struct i2c_adapter *intel_gmbus_get_adapter(
struct drm_i915_private *dev_priv, unsigned port);
 extern void intel_gmbus_set_speed(struct i2c_adapter *adapter, int speed);
 extern void intel_gmbus_force_bit(struct i2c_adapter *adapter, bool force_bit);
-extern inline bool intel_gmbus_is_forced_bit(struct i2c_adapter *adapter)
+static bool intel_gmbus_is_forced_bit(struct i2c_adapter *adapter)
 {
return container_of(adapter, struct intel_gmbus, adapter)->force_bit;
 }
-- 
1.8.1.4

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


Re: [Intel-gfx] [PATCH] [TRIVIAL] Fix declaration of intel_gmbus_{is_forced_bit/is_port_falid} in i915 driver.

2013-05-03 Thread Daniel Vetter
On Fri, May 03, 2013 at 03:03:37PM +0300, Jani Nikula wrote:
> On Fri, 03 May 2013, Daniel Vetter  wrote:
> > On Fri, May 03, 2013 at 11:17:42AM +0200, dl...@gmx.de wrote:
> >> From: Jan-Simon Möller 
> >> 
> >> Description:
> >> intel_gmbus_is_forced_bit is no extern as its body is right below.
> >> Likewise for intel_gmbus_is_port_valid.
> >> 
> >> This fixes a compilation issue with clang. An initial version of this patch
> >> was developed by PaX Team .
> >> This is respin of this patch.
> >> 
> >> Signed-off-by: Jan-Simon Möller 
> >> CC: pagee...@freemail.hu
> >> CC: daniel.vet...@ffwll.ch
> >> CC: airl...@linux.ie
> >> CC: intel-gfx@lists.freedesktop.org
> >> CC: dri-de...@lists.freedesktop.org
> >> CC: linux-ker...@vger.kernel.org
> > Picked up for -fixes, thanks for the patch.
> 
> Please drop it.
> 
> The patch removes the inline keyword, creating dozens of copies of the
> functions, and consequently loads of warnings:
> 
> drivers/gpu/drm/i915/i915_drv.h:1803:13: warning: ‘intel_gmbus_is_port_valid’ 
> defined but not used [-Wunused-function]
> drivers/gpu/drm/i915/i915_drv.h:1812:13: warning: ‘intel_gmbus_is_forced_bit’ 
> defined but not used [-Wunused-function]

Meh, rather embarrassing patch reading fail here. Dropped.

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] [git pull] drm for 3.10-rc1

2013-05-03 Thread Daniel Vetter
On Fri, May 3, 2013 at 4:39 PM, Josh Boyer  wrote:
> On Fri, May 03, 2013 at 02:25:57AM +0100, Dave Airlie wrote:
>>The following changes since commit b6a9b7f6b1f21735a7456d534dc0e68e61359d2c:
>>
>>  mm: prevent mmap_cache race in find_vma() (2013-04-04 11:46:28 -0700)
>>
>>are available in the git repository at:
>>
>>  git://people.freedesktop.org/~airlied/linux.git drm-next
>>
>>for you to fetch changes up to 307b9c022720f9de90d58e51743e01e9a42aec59:
>>
>>  qxl: update to new idr interfaces. (2013-05-03 10:37:20 +1000)
>>
>>
>
> So something in this batch of commits breaks a Macbook Pro Retina I have
> sitting here.  If I boot a Fedora kernel based on  Linux
> v3.9-8153-g5a148af, things boot up as they did previously with 3.9.0 and
> are generally working fine.  If I boot with a kernel based on Linux
> v3.9-8933-gce85722, it will boot but as soon as plymouth starts, I get
> nothing but static on the screen (like 1950s TV static).
>
> This machine is using i915 graphics, so I booted with nomodeset and did
> the 'modprobe drm debug=15; modprobe i915 modeset=1' trick and it
> repeated the failure with that.  I've gathered a bunch of data like
> dmesg, an intel_reg_snapshot, etc.  It's all attached below.
>
> I'll start bisecting to see if I can narrow down the commit that broke
> things, but I thought I would give a heads up now in case anyone has any
> ideas.

Looked through the log files and didn't see a clue. And usually DP
tends to fail pretty noisily. So I think the bisect result would be
interestin. Meanwhile:
- do you have drm.debug=0xe dmesg for a working kernel, too?
- is the panel completely dead or is just the backlight on (or panel
on but backlight off)?

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: force full modeset if the connector is in DPMS OFF mode

2013-05-03 Thread Daniel Vetter
On Fri, May 3, 2013 at 2:22 PM, Chris Wilson  wrote:
> On Fri, May 03, 2013 at 02:55:45PM +0300, Imre Deak wrote:
>> On Fri, 2013-05-03 at 12:44 +0100, Chris Wilson wrote:
>> > On Fri, May 03, 2013 at 02:22:09PM +0300, Imre Deak wrote:
>> > > Currently the driver's assumed behavior for a modeset with an attached
>> > > FB is that the corresponding connector will be switched to DPMS ON mode
>> > > if it happened to be in DPMS OFF (or another power save mode). This
>> > > wasn't enforced though if only the FB changed, everything else (format,
>> > > connector etc.) remaining the same. In this case we only set the new FB
>> > > base and left the connector in the old power save mode.
>> > >
>> > > Fix this by forcing a full modeset whenever there is an attached FB and
>> > > any affected connector is in a power save mode.
>> > >
>> > > Signed-off-by: Imre Deak 
>> >
>> > NAK. Papering over bugs, much?
>>
>> Ok. I posted this based on our IRC discussion where we seemed to agree
>> that DPMS_ON is assumed already; just that it's not done consistently.
>> That is if user space does a modeset now that ends up in a full modeset
>> (lets say a new FB with different format) then the kernel will do a
>> DPMS_ON. But if the new FB happens to be the same format then it won't.
>> I thought this is inconsistent and should be fixed.
>>
>> Another way to make it consistent would be then to remove DPMS ON from
>> the full modeset path..
>
> Right, we have mentioned in the past that the conflation between DPMS
> and modesetting is a more recent bug that is going to cause us even more
> trouble later. I am not sure how we can fix that as UXA already makes
> many bad assumptions.

Afaict the conflagration of dpms on after a modeset has been even in
the old crtc helpers. The only difference is that now we explicitly
update the dpms state tracking to avoid double-encoder enables. That
part allowed us to rip out tons of state tracking from tons of places.

So ever since kms happened, we have inconsistent setcrtc semantics.
Stopping to force dpms on will horribly break existing userspace, so
we can't do this. Making sure that the semantics are more consistent
(especially if we further optimize modeset transitions for fastboot)
seems like the lesser of two evils.

We'll get a 2nd chance to get this right the atomic modeset ioctl.
Until then, I want this duct-taped together, and Imre's patch looks
like a viable approach. Working beats ugly semantics here imo.

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


Re: [Intel-gfx] [PATCH v2] drm/i915: unreference default context on module unload

2013-05-03 Thread Ben Widawsky
On Fri, May 03, 2013 at 04:29:08PM +0300, Mika Kuoppala wrote:
> Before module unload is called, gpu_idle() will switch
> to default context. This will increment ref count of base
> object as the default context is 'running' on module unload
> time. Unreference the drm object so that when context
> is freed, base object is freed as well.
> 
> v2: added comment to explain the refcounts (Ben Widawsky)
> 
> Signed-off-by: Mika Kuoppala 

I left an r-b on the other patch. You should feel free to add it if you
actually do what I say.

Reviewed-by: Ben Widawsky 
[snip]
-- 
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] drm/i915: Add private api for power well usage -- alignment between graphic team and audio team

2013-05-03 Thread Wang, Xingchao
Hi Takashi,

> -Original Message-
> From: Takashi Iwai [mailto:ti...@suse.de]
> Sent: Friday, May 03, 2013 10:27 PM
> To: Barnes, Jesse
> Cc: Daniel Vetter; Wang, Xingchao; Li, Jocelyn; Daniel Vetter; Zanoni, Paulo 
> R;
> ville.syrj...@linux.intel.com; Lin, Mengdong; Girdwood, Liam R;
> intel-gfx@lists.freedesktop.org; alsa-de...@alsa-project.org; Wang Xingchao;
> Wysocki, Rafael J; Hindman, Gavin
> Subject: Re: [PATCH] drm/i915: Add private api for power well usage --
> alignment between graphic team and audio team
> 
> At Mon, 29 Apr 2013 08:02:19 -0700,
> Jesse Barnes wrote:
> >
> > On Sat, 27 Apr 2013 13:35:29 +0200
> > Daniel Vetter  wrote:
> >
> > > On Sat, Apr 27, 2013 at 09:20:39AM +, Wang, Xingchao wrote:
> > > > Let me throw a basic proposal on Audio driver side,  please give your
> comments freely.
> > > >
> > > > it contains the power well control usage points:
> > > > #1: audio request power well at boot up.
> > > > I915 may shut down power well after bootup initialization, as there's no
> monitor connected outside or only eDP on pipe A.
> > > > #2: audio request power on resume
> > > > After exit from D3 mode, audio driver quest power on. This may happen at
> normal resume or runtime resume.
> > > > #3: audio release power well control at suspend Audio driver will
> > > > let i915 know it doensot need power well anymore as it's going to 
> > > > suspend.
> This may happened at normal suspend or runtime suspend point.
> > > > #4: audio release power well when module unload Audio release
> > > > power well at remove callback to let i915 know.
> > >
> > > I miss the power well grab/dropping at runtime from the audio side.
> > > If the audio driver forces the power well to be on the entire time
> > > it's loaded, that's not good, since the power well stuff is very much for
> runtime PM.
> > > We _must_ be able to switch off the power well whenever possible.
> >
> > Xingchao, I'm not an audio developer so I'm probably way off.
> >
> > But what we really need is a very small and targeted set of calls into
> > the i915 driver from say the HDMI driver in HDA.  It looks like the
> > prepare/cleanup pair in the pcm_ops structure might be the right place
> > to put things?  If that's too fine grained, you could do it at
> > open/close time I guess, but the danger there is that some app will
> > keep the device open even while it's not playing.
> 
> Well, the question is what impact the power well on/off has against the audio.
> Do we need to resume the HD-audio controller / codec fully from the scratch?
> I guess not, but I have no certain idea.

Both the display H-DA controller and codec are under control by power well.
When the power well is off, for H-DA controller, the MMIO space is off, the PCI
Registers are in on well. The codec could not access anymore.

> 
> If the impact of the change (i.e. the procedure needed to resume) is small,
> somehow limited to the targeted converter/pin, it can be implemented in the
> prepare/cleanup callback of the codec driver, yes.
> 
> Though, the easiest path would be to add i915_get/put_power_well() in the
> codec probe, suspend, resume, and free callbacks, as you pointed out below.

Yes, and Jesse should get the background that, even power well is requested at 
probe,
It will not take long time to waste power. The controller/codec will enter 
power save mode
If runtime pm enabled. 

Thanks
--xingchao
> 
> > If that won't work, maybe calling i915 from hda_power_work in the
> > higher level code would be better?
> >
> > For detecting whether to call i915 at all, you can filter on the PCI
> > IDs (just look for an Intel graphics device and if present, try to get
> > the i915 symbols for the power functions).
> >
> > --- a/sound/pci/hda/hda_codec.c
> > +++ b/sound/pci/hda/hda_codec.c
> > @@ -3860,6 +3860,8 @@ static unsigned int hda_call_codec_suspend(struct
> hda_code
> > codec->patch_ops.suspend(codec);
> > hda_cleanup_all_streams(codec);
> > state = hda_set_power_state(codec, AC_PWRST_D3);
> > +   if (i915_shared_power_well)
> > +   i915_put_power_well(codec->i915_data);
> > /* Cancel delayed work if we aren't currently running from it. */
> > if (!in_wq)
> > cancel_delayed_work_sync(&codec->power_work);
> > @@ -4807,6 +4809,9 @@ static void __snd_hda_power_up(struct
> hda_codec *codec, bo
> > return;
> > spin_unlock(&codec->power_lock);
> >
> > +   if (i915_shared_power_well)
> > +   i915_get_power_well(codec->i915_data);
> > +
> > cancel_delayed_work_sync(&codec->power_work);
> >
> > spin_lock(&codec->power_lock);
> >
> > With some code at init time to get the i915 symbols you need to call
> > and whether or not the shared power well is present...
> >
> > Takashi, any other ideas?
> >
> > The high level goal here should be for the audio driver to call into
> > i915 with get/put power well around the sequences where

[Intel-gfx] [PATCH 5/5] drm/i915: only disable DDI sound if intel_crtc->eld_vld

2013-05-03 Thread Paulo Zanoni
From: Paulo Zanoni 

We already have the same check on intel_enable_ddi. This patch
prevents "unclaimed register" messages when the power well is
disabled.

V2: Reset intel_crtc->eld_vld to false after the mode_set function.
V3: Add both "type != INTEL_OUTPUT_EDP" requested.

Signed-off-by: Paulo Zanoni 
Reviewed-by: Damien Lespiau 
---
 drivers/gpu/drm/i915/intel_ddi.c |   11 +++
 drivers/gpu/drm/i915/intel_display.c |2 +-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 3ff4de6..21fb852 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1300,7 +1300,7 @@ static void intel_enable_ddi(struct intel_encoder 
*intel_encoder)
ironlake_edp_backlight_on(intel_dp);
}
 
-   if (intel_crtc->eld_vld) {
+   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);
@@ -1318,9 +1318,12 @@ static void intel_disable_ddi(struct intel_encoder 
*intel_encoder)
struct drm_i915_private *dev_priv = dev->dev_private;
uint32_t tmp;
 
-   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) {
+   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 (type == INTEL_OUTPUT_EDP) {
struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index fcb1367..7aa2295 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3855,8 +3855,8 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
/* crtc should still be enabled when we disable it. */
WARN_ON(!crtc->enabled);
 
-   intel_crtc->eld_vld = false;
dev_priv->display.crtc_disable(crtc);
+   intel_crtc->eld_vld = false;
intel_crtc_update_sarea(crtc, false);
dev_priv->display.off(crtc);
 
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 4/5] drm/i915: check the power well on i915_pipe_enabled

2013-05-03 Thread Paulo Zanoni
From: Paulo Zanoni 

This fixes "unclaimed register" messages when the power well is
disabled and there's a GPU hang.

v2: Use the new intel_display_power_enabled().
v3: Use the new domains for intel_display_power_enabled().

Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/i915_irq.c |4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 03a31be..161101f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -384,6 +384,10 @@ i915_pipe_enabled(struct drm_device *dev, int pipe)
enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
  pipe);
 
+   if (!intel_display_power_enabled(dev,
+   POWER_DOMAIN_TRANSCODER(cpu_transcoder)))
+   return false;
+
return I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_ENABLE;
 }
 
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 3/5] drm/i915: clear FPGA_DBG_RM_NOCLAIM when capturing error state

2013-05-03 Thread Paulo Zanoni
From: Paulo Zanoni 

In the error state function we read the registers without checking if
the power well is on, so after doing this we have to clear the
FPGA_DBG_RM_NOCLAIM bit to prevent the next I915_WRITE from detecting
it and printing an error message.

The first version of this patch was checking for the power well state
and then avoiding reading registers that were off, but the reviewers
requested to just read the registers any way and then later clear the
FPGA_DBG_RM_NOCLAIM bit.

Signed-off-by: Paulo Zanoni 
Reviewed-by: Damien Lespiau 
---
 drivers/gpu/drm/i915/intel_display.c |7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 190b787..fcb1367 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9826,6 +9826,13 @@ intel_display_capture_error_state(struct drm_device *dev)
error->pipe[i].vsync = I915_READ(VSYNC(cpu_transcoder));
}
 
+   /* In the code above we read the registers without checking if the power
+* well was on, so here we have to clear the FPGA_DBG_RM_NOCLAIM bit to
+* prevent the next I915_WRITE from detecting it and printing an error
+* message. */
+   if (HAS_POWER_WELL(dev))
+   I915_WRITE_NOTRACE(FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
+
return error;
 }
 
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 2/5] drm/i915: add power well and cpu transcoder info to the error state

2013-05-03 Thread Paulo Zanoni
From: Paulo Zanoni 

We need to dump these registers if we want to properly interpret the
others.

Signed-off-by: Paulo Zanoni 
Reviewed-by: Damien Lespiau 
---
 drivers/gpu/drm/i915/intel_display.c |   13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 71a9770..190b787 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9740,6 +9740,9 @@ int intel_modeset_vga_set_state(struct drm_device *dev, 
bool state)
 #include 
 
 struct intel_display_error_state {
+
+   u32 power_well_driver;
+
struct intel_cursor_error_state {
u32 control;
u32 position;
@@ -9748,6 +9751,7 @@ struct intel_display_error_state {
} cursor[I915_MAX_PIPES];
 
struct intel_pipe_error_state {
+   enum transcoder cpu_transcoder;
u32 conf;
u32 source;
 
@@ -9782,8 +9786,12 @@ intel_display_capture_error_state(struct drm_device *dev)
if (error == NULL)
return NULL;
 
+   if (HAS_POWER_WELL(dev))
+   error->power_well_driver = I915_READ(HSW_PWR_WELL_DRIVER);
+
for_each_pipe(i) {
cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv, i);
+   error->pipe[i].cpu_transcoder = cpu_transcoder;
 
if (INTEL_INFO(dev)->gen <= 6 || IS_VALLEYVIEW(dev)) {
error->cursor[i].control = I915_READ(CURCNTR(i));
@@ -9829,8 +9837,13 @@ intel_display_print_error_state(struct seq_file *m,
int i;
 
seq_printf(m, "Num Pipes: %d\n", INTEL_INFO(dev)->num_pipes);
+   if (HAS_POWER_WELL(dev))
+   seq_printf(m, "PWR_WELL_CTL2: %08x\n",
+  error->power_well_driver);
for_each_pipe(i) {
seq_printf(m, "Pipe [%d]:\n", i);
+   seq_printf(m, "  CPU transcoder: %c\n",
+  transcoder_name(error->pipe[i].cpu_transcoder));
seq_printf(m, "  CONF: %08x\n", error->pipe[i].conf);
seq_printf(m, "  SRC: %08x\n", error->pipe[i].source);
seq_printf(m, "  HTOTAL: %08x\n", error->pipe[i].htotal);
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 1/5] drm/i915: add intel_display_power_enabled

2013-05-03 Thread Paulo Zanoni
From: Paulo Zanoni 

This should replace intel_using_power_well. The idea is that we're
adding the requested power domain as an argument, so this might enable
the code to look less platform-specific and also allows us to easily
add new domains in case we need.

v2: Add more domains to enum intel_display_power_domain
v3: Even more domains requested

Requested-by: Daniel Vetter 
Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/i915_drv.h  |   18 ++
 drivers/gpu/drm/i915/intel_display.c |   11 ++-
 drivers/gpu/drm/i915/intel_drv.h |3 ++-
 drivers/gpu/drm/i915/intel_pm.c  |   24 
 4 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3ac71db..5160e1e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -88,6 +88,24 @@ enum port {
 };
 #define port_name(p) ((p) + 'A')
 
+enum intel_display_power_domain {
+   POWER_DOMAIN_PIPE_A,
+   POWER_DOMAIN_PIPE_B,
+   POWER_DOMAIN_PIPE_C,
+   POWER_DOMAIN_PIPE_A_PANEL_FITTER,
+   POWER_DOMAIN_PIPE_B_PANEL_FITTER,
+   POWER_DOMAIN_PIPE_C_PANEL_FITTER,
+   POWER_DOMAIN_TRANSCODER_A,
+   POWER_DOMAIN_TRANSCODER_B,
+   POWER_DOMAIN_TRANSCODER_C,
+   POWER_DOMAIN_TRANSCODER_EDP = POWER_DOMAIN_TRANSCODER_A + 0xF,
+};
+
+#define POWER_DOMAIN_PIPE(pipe) ((pipe) + POWER_DOMAIN_PIPE_A)
+#define POWER_DOMAIN_PIPE_PANEL_FITTER(pipe) \
+   ((pipe) + POWER_DOMAIN_PIPE_A_PANEL_FITTER)
+#define POWER_DOMAIN_TRANSCODER(tran) ((tran) + POWER_DOMAIN_TRANSCODER_A)
+
 enum hpd_pin {
HPD_NONE = 0,
HPD_PORT_A = HPD_NONE, /* PORT_A is internal */
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 6504337..71a9770 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1110,8 +1110,8 @@ void assert_pipe(struct drm_i915_private *dev_priv,
if (pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE)
state = true;
 
-   if (!intel_using_power_well(dev_priv->dev) &&
-   cpu_transcoder != TRANSCODER_EDP) {
+   if (!intel_display_power_enabled(dev_priv->dev,
+   POWER_DOMAIN_TRANSCODER(cpu_transcoder))) {
cur_state = false;
} else {
reg = PIPECONF(cpu_transcoder);
@@ -3523,7 +3523,8 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
/* XXX: Once we have proper panel fitter state tracking implemented with
 * hardware state read/check support we should switch to only disable
 * the panel fitter when we know it's used. */
-   if (intel_using_power_well(dev)) {
+   if (intel_display_power_enabled(dev,
+   POWER_DOMAIN_PIPE_PANEL_FITTER(pipe))) {
I915_WRITE(PF_CTL(pipe), 0);
I915_WRITE(PF_WIN_SZ(pipe), 0);
}
@@ -6023,8 +6024,8 @@ static bool haswell_get_pipe_config(struct intel_crtc 
*crtc,
enum transcoder cpu_transcoder = crtc->config.cpu_transcoder;
uint32_t tmp;
 
-   if (!intel_using_power_well(dev_priv->dev) &&
-   cpu_transcoder != TRANSCODER_EDP)
+   if (!intel_display_power_enabled(dev,
+   POWER_DOMAIN_TRANSCODER(cpu_transcoder)))
return false;
 
tmp = I915_READ(PIPECONF(cpu_transcoder));
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index dfcf546..ae73631 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -759,7 +759,8 @@ extern void intel_update_fbc(struct drm_device *dev);
 extern void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
 extern void intel_gpu_ips_teardown(void);
 
-extern bool intel_using_power_well(struct drm_device *dev);
+extern bool intel_display_power_enabled(struct drm_device *dev,
+   enum intel_display_power_domain domain);
 extern void intel_init_power_well(struct drm_device *dev);
 extern void intel_set_power_well(struct drm_device *dev, bool enable);
 extern void intel_enable_gt_powersave(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0f4b46e..664c59f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4333,15 +4333,31 @@ void intel_init_clock_gating(struct drm_device *dev)
  * enable it, so check if it's enabled and also check if we've requested it to
  * be enabled.
  */
-bool intel_using_power_well(struct drm_device *dev)
+bool intel_display_power_enabled(struct drm_device *dev,
+enum intel_display_power_domain domain)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
 
-   if (IS_HASWELL(dev))
+   if (!HAS_POWER_WELL(dev))
+   return true;
+
+   switch (domain) {

Re: [Intel-gfx] [PATCH] drm/i915: fix Haswell pfit power well check v2

2013-05-03 Thread Paulo Zanoni
2013/5/3 Mika Kuoppala :
> Jesse Barnes  writes:
>
>> We can't read the pfit regs if the power well is off, so use the cached
>> value.
>>
>> v2: re-add lost comment (Jesse)
>> make sure the crtc using the fitter is actually enabled (Jesse)
>>
>> Signed-off-by: Jesse Barnes 
>> ---
>>  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 6504337..6be34f2 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -5918,7 +5918,7 @@ static void haswell_modeset_global_resources(struct 
>> drm_device *dev)
>>* sequence that's not yet available. Just in case desktop eDP
>>* on PORT D is possible on haswell, too. */
>>   /* Even the eDP panel fitter is outside the always-on well. */
>> - if (I915_READ(PF_WIN_SZ(crtc->pipe)))
>> + if (crtc->config.pch_pfit.size && crtc->base.enabled)
>>   enable = true;
>>   }
>>
>
> Remove the now useless *dev_priv to remove compiler warning and then add
>
> Reviewed-by: Mika Kuoppala 

Yay, dmesg is clean again with this patch + Daniel's patch 06 + my
local patches which I'll resend today.

With the warn pointed by Mika removed:
Reviewed-by: Paulo Zanoni 
Tested-by: Paulo Zanoni 

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



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


Re: [Intel-gfx] [PATCH 6/6] drm/i915: fix up adjusted_mode tracking for interlaced modes

2013-05-03 Thread Paulo Zanoni
2013/5/3 Daniel Vetter :
> On Fri, May 3, 2013 at 11:49 AM, Daniel Vetter  wrote:
>> With the hw state readout&check code it's important that the values we
>> keep around are the canonical ones. Unfortunately when adding the pipe
>> timings readout support I've missed that the write side adjusts the
>> timings in the pipe config.
>>
>> Fix this up.
>>
>> Reported-by: Paulo Zanoni 
>> Signed-off-by: Daniel Vetter 
>
> I better be a good example ;-) So:
>
> This fixes a regression introduced in
>
> commit 1bd1bd806037af04dd1d7bdd39b2b04090c10d2c
> Author: Daniel Vetter 
> Date:   Mon Apr 29 21:56:12 2013 +0200
>
> drm/i915: hw state readout support for pipe timings
>
> Cc: Mika Kuoppala 

To make it super complete, you could also explicitly mention that this
fixes a WARN when we use interlaced modes :)

So now I booted it with the eDP + DP-interlaced I was using yesterday
and I don't see the error message anymore.

Reviewed-by: Paulo Zanoni 
Tested-by: Paulo Zanoni 


>
> Cheers, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch



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


Re: [Intel-gfx] [alsa-devel] [PATCH] drm/i915: Add private api for power well usage -- alignment between graphic team and audio team

2013-05-03 Thread Takashi Iwai
At Fri, 03 May 2013 14:02:04 +0200,
David Henningsson wrote:
> 
> On 05/03/2013 10:28 AM, Wang, Xingchao wrote:
> > Hi David,
> >
> > Thank you very much for your draft patch.
> > I have some more work on a new patchset, some ideas are from your patch.
> 
> Thanks.
> 
> > Here's a brief introduction of attached patchset:
> >
> > 1. a new bus type in /sound/had_bus.c, used to bind the single module and 
> > codec device
> > It looks like ac97_bus.c
> 
> I don't understand why this is needed. It does not look like it's used 
> from the gfx side either, or anything like that?
>
> > 2. add a new device node in "struct hda_codec", it's used to register for 
> > new bus type.
> >
> > 3. a new single module hdmi_i915, which compiled in only when DRM_I915 and 
> > CODEC_HDMI enabled.
> > It stores the private API for gfx part.
> > There's no support to probe haswell hdmi codec only yet. Power well will be 
> > used only for haswell display audio.
> >
> > 4. power well API implementation in gfx side.
> >
> > Please feel free to add your idea and I will help test your patch too.
> 
> Ok. So the patch I wrote would (if it works) be combined with your patch 
> 3, which implements the gfx side. The gfx side is not my area of expertise.
> 
> The proposed way in my patch would be more elegant since it does not 
> introduce any i915 related code in hda_codec* files.
> 
> Still, Takashi is the boss here so he has the final say :-)

Indeed.  If the reference to power well API is limited in a newly
split snd-hda-codec-hdmi-i915 driver, we don't have to create yet
another driver instance.  The snd-hda-codec-hdmi-i915 can simply
depend on i915, by referring to a symbol exported from i915 driver.

If we now touch the whole PM sequence in both gfx and audio drivers
(i.e. it influences on the HD-audio controller code, hda_intel.c),
then we may need a different management.  But I thought it's not yet
discussed here, right?


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


Re: [Intel-gfx] [PATCH] drm/i915: Add private api for power well usage -- alignment between graphic team and audio team

2013-05-03 Thread Takashi Iwai
At Mon, 29 Apr 2013 08:02:19 -0700,
Jesse Barnes wrote:
> 
> On Sat, 27 Apr 2013 13:35:29 +0200
> Daniel Vetter  wrote:
> 
> > On Sat, Apr 27, 2013 at 09:20:39AM +, Wang, Xingchao wrote:
> > > Let me throw a basic proposal on Audio driver side,  please give your 
> > > comments freely.
> > > 
> > > it contains the power well control usage points:
> > > #1: audio request power well at boot up.
> > > I915 may shut down power well after bootup initialization, as there's no 
> > > monitor connected outside or only eDP on pipe A.
> > > #2: audio request power on resume
> > > After exit from D3 mode, audio driver quest power on. This may happen at 
> > > normal resume or runtime resume.
> > > #3: audio release power well control at suspend
> > > Audio driver will let i915 know it doensot need power well anymore as 
> > > it's going to suspend. This may happened at normal suspend or runtime 
> > > suspend point.
> > > #4: audio release power well when module unload
> > > Audio release power well at remove callback to let i915 know.
> > 
> > I miss the power well grab/dropping at runtime from the audio side. If the
> > audio driver forces the power well to be on the entire time it's loaded,
> > that's not good, since the power well stuff is very much for runtime PM.
> > We _must_ be able to switch off the power well whenever possible.
> 
> Xingchao, I'm not an audio developer so I'm probably way off.
> 
> But what we really need is a very small and targeted set of calls into
> the i915 driver from say the HDMI driver in HDA.  It looks like the
> prepare/cleanup pair in the pcm_ops structure might be the right place
> to put things?  If that's too fine grained, you could do it at
> open/close time I guess, but the danger there is that some app will
> keep the device open even while it's not playing.

Well, the question is what impact the power well on/off has against
the audio.  Do we need to resume the HD-audio controller / codec fully
from the scratch?  I guess not, but I have no certain idea.

If the impact of the change (i.e. the procedure needed to resume) is
small, somehow limited to the targeted converter/pin, it can be
implemented in the prepare/cleanup callback of the codec driver, yes.

Though, the easiest path would be to add i915_get/put_power_well() in
the codec probe, suspend, resume, and free callbacks, as you pointed
out below.

> If that won't work, maybe calling i915 from hda_power_work in the
> higher level code would be better?
> 
> For detecting whether to call i915 at all, you can filter on the PCI
> IDs (just look for an Intel graphics device and if present, try to get
> the i915 symbols for the power functions).
> 
> --- a/sound/pci/hda/hda_codec.c
> +++ b/sound/pci/hda/hda_codec.c
> @@ -3860,6 +3860,8 @@ static unsigned int hda_call_codec_suspend(struct 
> hda_code
> codec->patch_ops.suspend(codec);
> hda_cleanup_all_streams(codec);
> state = hda_set_power_state(codec, AC_PWRST_D3);
> +   if (i915_shared_power_well)
> +   i915_put_power_well(codec->i915_data);
> /* Cancel delayed work if we aren't currently running from it. */
> if (!in_wq)
> cancel_delayed_work_sync(&codec->power_work);
> @@ -4807,6 +4809,9 @@ static void __snd_hda_power_up(struct hda_codec *codec, 
> bo
> return;
> spin_unlock(&codec->power_lock);
>  
> +   if (i915_shared_power_well)
> +   i915_get_power_well(codec->i915_data);
> +
> cancel_delayed_work_sync(&codec->power_work);
>  
> spin_lock(&codec->power_lock);
> 
> With some code at init time to get the i915 symbols you need to call
> and whether or not the shared power well is present...
> 
> Takashi, any other ideas?
> 
> The high level goal here should be for the audio driver to call into
> i915 with get/put power well around the sequences where it needs the
> power to be up (reading/writing registers, playing audio), but not
> across the whole time the driver is loaded, just like you already do
> with the powersave work functions, e.g. hda_call_codec_suspend.

I guess controlling the suspend/resume path would be practically
working well.  If a system is really power-conscious, it should use a
sound backend like PulseAudio, which closes the unused PCM devices
frequently enough, and the power_save option should be changed by the
power management tool on the fly.

If such mechanisms aren't used, it means that user doesn't care about
power, after all.


thanks,

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


Re: [Intel-gfx] [PATCH 2/6] drm/i915: PCH_ prefix for transcoder timings

2013-05-03 Thread Paulo Zanoni
2013/5/3 Daniel Vetter :
> While at it, also extract a common helper to copy the timings from the
> cpu transcoder to the pch transcoder. That way it's really explicit
> how the lpt transcoder is hardcoded.
>
> v2:
> - Re-align #defines properly (Paulo).
> - Use cpu_transcoder when copying pipe timings (Paulo).
> - s/intel_pch_transcoder_enable/intel_pch_transcoder_set_timings/
>   since we already have a pch transcoder enable function, and this is
>   clearer, too.
> - Fixup 80 char line overflow in intel_display.c. I've opted to ignore
>   this in i915_reg.h and i915_ums.c since meh.
>
> Cc: Paulo Zanoni 
> Signed-off-by: Daniel Vetter 

Reviewed-by: Paulo Zanoni 

> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 70 
> ++--
>  drivers/gpu/drm/i915/i915_ums.c  | 48 -
>  drivers/gpu/drm/i915/intel_display.c | 42 +-
>  3 files changed, 85 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d6fc8e7..66c906d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3925,25 +3925,25 @@
>
>  /* transcoder */
>
> -#define _TRANS_HTOTAL_A  0xe
> -#define  TRANS_HTOTAL_SHIFT 16
> -#define  TRANS_HACTIVE_SHIFT0
> -#define _TRANS_HBLANK_A  0xe0004
> -#define  TRANS_HBLANK_END_SHIFT 16
> -#define  TRANS_HBLANK_START_SHIFT 0
> -#define _TRANS_HSYNC_A   0xe0008
> -#define  TRANS_HSYNC_END_SHIFT  16
> -#define  TRANS_HSYNC_START_SHIFT 0
> -#define _TRANS_VTOTAL_A  0xe000c
> -#define  TRANS_VTOTAL_SHIFT 16
> -#define  TRANS_VACTIVE_SHIFT0
> -#define _TRANS_VBLANK_A  0xe0010
> -#define  TRANS_VBLANK_END_SHIFT 16
> -#define  TRANS_VBLANK_START_SHIFT 0
> -#define _TRANS_VSYNC_A   0xe0014
> -#define  TRANS_VSYNC_END_SHIFT  16
> -#define  TRANS_VSYNC_START_SHIFT 0
> -#define _TRANS_VSYNCSHIFT_A0xe0028
> +#define _PCH_TRANS_HTOTAL_A0xe
> +#define  TRANS_HTOTAL_SHIFT16
> +#define  TRANS_HACTIVE_SHIFT   0
> +#define _PCH_TRANS_HBLANK_A0xe0004
> +#define  TRANS_HBLANK_END_SHIFT16
> +#define  TRANS_HBLANK_START_SHIFT  0
> +#define _PCH_TRANS_HSYNC_A 0xe0008
> +#define  TRANS_HSYNC_END_SHIFT 16
> +#define  TRANS_HSYNC_START_SHIFT   0
> +#define _PCH_TRANS_VTOTAL_A0xe000c
> +#define  TRANS_VTOTAL_SHIFT16
> +#define  TRANS_VACTIVE_SHIFT   0
> +#define _PCH_TRANS_VBLANK_A0xe0010
> +#define  TRANS_VBLANK_END_SHIFT16
> +#define  TRANS_VBLANK_START_SHIFT  0
> +#define _PCH_TRANS_VSYNC_A 0xe0014
> +#define  TRANS_VSYNC_END_SHIFT 16
> +#define  TRANS_VSYNC_START_SHIFT   0
> +#define _PCH_TRANS_VSYNCSHIFT_A0xe0028
>
>  #define _TRANSA_DATA_M1  0xe0030
>  #define _TRANSA_DATA_N1  0xe0034
> @@ -4021,22 +4021,22 @@
>  #define HSW_TVIDEO_DIP_VSC_DATA(trans) \
>  _TRANSCODER(trans, HSW_VIDEO_DIP_VSC_DATA_A, 
> HSW_VIDEO_DIP_VSC_DATA_B)
>
> -#define _TRANS_HTOTAL_B  0xe1000
> -#define _TRANS_HBLANK_B  0xe1004
> -#define _TRANS_HSYNC_B   0xe1008
> -#define _TRANS_VTOTAL_B  0xe100c
> -#define _TRANS_VBLANK_B  0xe1010
> -#define _TRANS_VSYNC_B   0xe1014
> -#define _TRANS_VSYNCSHIFT_B 0xe1028
> -
> -#define TRANS_HTOTAL(pipe) _PIPE(pipe, _TRANS_HTOTAL_A, _TRANS_HTOTAL_B)
> -#define TRANS_HBLANK(pipe) _PIPE(pipe, _TRANS_HBLANK_A, _TRANS_HBLANK_B)
> -#define TRANS_HSYNC(pipe) _PIPE(pipe, _TRANS_HSYNC_A, _TRANS_HSYNC_B)
> -#define TRANS_VTOTAL(pipe) _PIPE(pipe, _TRANS_VTOTAL_A, _TRANS_VTOTAL_B)
> -#define TRANS_VBLANK(pipe) _PIPE(pipe, _TRANS_VBLANK_A, _TRANS_VBLANK_B)
> -#define TRANS_VSYNC(pipe) _PIPE(pipe, _TRANS_VSYNC_A, _TRANS_VSYNC_B)
> -#define TRANS_VSYNCSHIFT(pipe) _PIPE(pipe, _TRANS_VSYNCSHIFT_A, \
> -_TRANS_VSYNCSHIFT_B)
> +#define _PCH_TRANS_HTOTAL_B  0xe1000
> +#define _PCH_TRANS_HBLANK_B  0xe1004
> +#define _PCH_TRANS_HSYNC_B   0xe1008
> +#define _PCH_TRANS_VTOTAL_B  0xe100c
> +#define _PCH_TRANS_VBLANK_B  0xe1010
> +#define _PCH_TRANS_VSYNC_B   0xe1014
> +#define _PCH_TRANS_VSYNCSHIFT_B 0xe1028
> +
> +#define PCH_TRANS_HTOTAL(pipe) _PIPE(pipe, _PCH_TRANS_HTOTAL_A, 
> _PCH_TRANS_HTOTAL_B)
> +#define PCH_TRANS_HBLANK(pipe) _PIPE(pipe, _PCH_TRANS_HBLANK_A, 
> _PCH_TRANS_HBLANK_B)
> +#define PCH_TRANS_HSYNC(pipe) _PIPE(pipe, _PCH_TRANS_HSYNC_A, 
> _PCH_TRANS_HSYNC_B)
> +#define PCH_TRANS_VTOTAL(pipe) _PIPE(pipe, _PCH_TRANS_VTOTAL_A, 
> _PCH_TRANS_VTOTAL_B)
> +#define PCH_TRANS_VBLANK(pipe) _PIPE(pipe, _PCH_TRANS_VBLANK_A, 
> _PCH_TRANS_VBLANK_B)
> +#define PCH_TRANS_VSYNC(pipe) _PIPE(pipe, _PCH_TRANS_VSYNC_A, 
> _PCH_TRANS_VSYNC_B)
> +#define PCH_TRANS_VSYNCSHIFT(pipe) _PIPE(pipe, _PCH_TRANS_VSYNCSHIFT_A, \
> +

[Intel-gfx] [i-g-t PATCH v2] kms_flip: make sure we are unblanked during the test

2013-05-03 Thread Imre Deak
This is a probable reason for some of the sporadic kms_flip failures.
One such is https://bugs.freedesktop.org/show_bug.cgi?id=59834.

v2:
- use unsigned long for KDSETMODE/KDGETMODE
- fix passing the parameter to KDGETMODE as it should be by value
- actually testing that it works..

Signed-off-by: Imre Deak 
---
 tests/kms_flip.c |   33 -
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 39f0043..601a4a1 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "i915_drm.h"
 #include "drmtest.h"
@@ -1072,6 +1073,13 @@ static void run_test_on_crtc(struct test_output *o, int 
crtc, int duration)
}
assert(fb_is_bound(o, o->fb_ids[0]));
 
+   /*
+* Make sure we're unblanked if for example the test was started with
+* the console blanked. In this case the kernel might do a DPMS_ON in
+* the above mode set call, but it's not guaranteed.
+*/
+   do_or_die(set_dpms(o, DRM_MODE_DPMS_ON));
+
/* quiescent the hw a bit so ensure we don't miss a single frame */
if (o->flags & TEST_CHECK_TS)
sleep(1);
@@ -1153,6 +1161,21 @@ static void get_timestamp_format(void)
monotonic_timestamp ? "monotonic" : "real");
 }
 
+static unsigned long set_vt_mode(unsigned long mode)
+{
+   int fd;
+   unsigned long prev_mode;
+
+   fd = open("/dev/tty0", O_RDONLY);
+   assert(fd >= 0);
+
+   prev_mode = 0;
+   drmIoctl(fd, KDGETMODE, &prev_mode);
+   drmIoctl(fd, KDSETMODE, (void *)mode);
+
+   return prev_mode;
+}
+
 int main(int argc, char **argv)
 {
struct {
@@ -1200,13 +1223,18 @@ int main(int argc, char **argv)
{ 1, TEST_FLIP | TEST_EINVAL | TEST_FB_BAD_TILING, 
"flip-vs-bad-tiling" },
};
int i;
+   bool dry_run;
+   unsigned long prev_vt_mode;
 
drmtest_subtest_init(argc, argv);
 
drm_fd = drm_open_any();
 
-   if (!drmtest_only_list_subtests())
+   dry_run = drmtest_only_list_subtests();
+   if (!dry_run) {
+   prev_vt_mode = set_vt_mode(KD_GRAPHICS);
get_timestamp_format();
+   }
 
bufmgr = drm_intel_bufmgr_gem_init(drm_fd, 4096);
devid = intel_get_drm_devid(drm_fd);
@@ -1219,6 +1247,9 @@ int main(int argc, char **argv)
}
}
 
+   if (!dry_run)
+   set_vt_mode(prev_vt_mode);
+
close(drm_fd);
 
return 0;
-- 
1.7.10.4

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


[Intel-gfx] [PATCH v2] drm/i915: unreference default context on module unload

2013-05-03 Thread Mika Kuoppala
Before module unload is called, gpu_idle() will switch
to default context. This will increment ref count of base
object as the default context is 'running' on module unload
time. Unreference the drm object so that when context
is freed, base object is freed as well.

v2: added comment to explain the refcounts (Ben Widawsky)

Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_gem_context.c |8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 9e8c685..280617e 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -274,6 +274,14 @@ void i915_gem_context_fini(struct drm_device *dev)
intel_gpu_reset(dev);
 
i915_gem_object_unpin(dctx->obj);
+
+   /* When default context is created and switched to, base object refcount
+* will be 2 (+1 from object creation and +1 from do_switch()).
+* i915_gem_context_fini() will be called after gpu_idle() has switched
+* to default context. So we need to unreference the base object once
+* to offset the do_switch part, so that i915_gem_context_unreference()
+* can then free the base object correctly. */
+   drm_gem_object_unreference(&dctx->obj->base);
i915_gem_context_unreference(dctx);
 }
 
-- 
1.7.9.5

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


Re: [Intel-gfx] [PATCH] drm/i915: fix Haswell pfit power well check v2

2013-05-03 Thread Mika Kuoppala
Jesse Barnes  writes:

> We can't read the pfit regs if the power well is off, so use the cached
> value.
>
> v2: re-add lost comment (Jesse)
> make sure the crtc using the fitter is actually enabled (Jesse)
>
> Signed-off-by: Jesse Barnes 
> ---
>  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 6504337..6be34f2 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5918,7 +5918,7 @@ static void haswell_modeset_global_resources(struct 
> drm_device *dev)
>* sequence that's not yet available. Just in case desktop eDP
>* on PORT D is possible on haswell, too. */
>   /* Even the eDP panel fitter is outside the always-on well. */
> - if (I915_READ(PF_WIN_SZ(crtc->pipe)))
> + if (crtc->config.pch_pfit.size && crtc->base.enabled)
>   enable = true;
>   }
>

Remove the now useless *dev_priv to remove compiler warning and then add

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


[Intel-gfx] [i-g-t PATCH] kms_flip: make sure we are unblanked during the test

2013-05-03 Thread Imre Deak
This is a probable reason for some of the sporadic kms_flip failures.
One such is https://bugs.freedesktop.org/show_bug.cgi?id=59834.

Signed-off-by: Imre Deak 
---
 tests/kms_flip.c |   32 +++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 39f0043..34c53b7 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "i915_drm.h"
 #include "drmtest.h"
@@ -1072,6 +1073,13 @@ static void run_test_on_crtc(struct test_output *o, int 
crtc, int duration)
}
assert(fb_is_bound(o, o->fb_ids[0]));
 
+   /*
+* Make sure we're unblanked if for example the test was started with
+* the console blanked. In this case the kernel might do a DPMS_ON in
+* the above mode set call, but it's not guaranteed.
+*/
+   do_or_die(set_dpms(o, DRM_MODE_DPMS_ON));
+
/* quiescent the hw a bit so ensure we don't miss a single frame */
if (o->flags & TEST_CHECK_TS)
sleep(1);
@@ -1153,6 +1161,20 @@ static void get_timestamp_format(void)
monotonic_timestamp ? "monotonic" : "real");
 }
 
+static unsigned set_vt_mode(unsigned mode)
+{
+   int fd;
+   unsigned long prev_mode;
+
+   fd = open("/dev/tty0", O_RDONLY);
+   assert(fd >= 0);
+
+   drmIoctl(fd, KDGETMODE, &prev_mode);
+   drmIoctl(fd, KDSETMODE, &mode);
+
+   return prev_mode;
+}
+
 int main(int argc, char **argv)
 {
struct {
@@ -1200,13 +1222,18 @@ int main(int argc, char **argv)
{ 1, TEST_FLIP | TEST_EINVAL | TEST_FB_BAD_TILING, 
"flip-vs-bad-tiling" },
};
int i;
+   bool dry_run;
+   unsigned prev_vt_mode;
 
drmtest_subtest_init(argc, argv);
 
drm_fd = drm_open_any();
 
-   if (!drmtest_only_list_subtests())
+   dry_run = drmtest_only_list_subtests();
+   if (!dry_run) {
+   prev_vt_mode = set_vt_mode(KD_GRAPHICS);
get_timestamp_format();
+   }
 
bufmgr = drm_intel_bufmgr_gem_init(drm_fd, 4096);
devid = intel_get_drm_devid(drm_fd);
@@ -1219,6 +1246,9 @@ int main(int argc, char **argv)
}
}
 
+   if (!dry_run)
+   set_vt_mode(prev_vt_mode);
+
close(drm_fd);
 
return 0;
-- 
1.7.10.4

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


Re: [Intel-gfx] [PATCH] drm/i915: force full modeset if the connector is in DPMS OFF mode

2013-05-03 Thread Imre Deak
On Fri, 2013-05-03 at 13:22 +0100, Chris Wilson wrote:
> On Fri, May 03, 2013 at 02:55:45PM +0300, Imre Deak wrote:
> > On Fri, 2013-05-03 at 12:44 +0100, Chris Wilson wrote:
> > > On Fri, May 03, 2013 at 02:22:09PM +0300, Imre Deak wrote:
> > > > Currently the driver's assumed behavior for a modeset with an attached
> > > > FB is that the corresponding connector will be switched to DPMS ON mode
> > > > if it happened to be in DPMS OFF (or another power save mode). This
> > > > wasn't enforced though if only the FB changed, everything else (format,
> > > > connector etc.) remaining the same. In this case we only set the new FB
> > > > base and left the connector in the old power save mode.
> > > > 
> > > > Fix this by forcing a full modeset whenever there is an attached FB and
> > > > any affected connector is in a power save mode.
> > > > 
> > > > Signed-off-by: Imre Deak 
> > > 
> > > NAK. Papering over bugs, much?
> > 
> > Ok. I posted this based on our IRC discussion where we seemed to agree
> > that DPMS_ON is assumed already; just that it's not done consistently.
> > That is if user space does a modeset now that ends up in a full modeset
> > (lets say a new FB with different format) then the kernel will do a
> > DPMS_ON. But if the new FB happens to be the same format then it won't.
> > I thought this is inconsistent and should be fixed.
> > 
> > Another way to make it consistent would be then to remove DPMS ON from
> > the full modeset path..
> 
> Right, we have mentioned in the past that the conflation between DPMS
> and modesetting is a more recent bug that is going to cause us even more
> trouble later. I am not sure how we can fix that as UXA already makes
> many bad assumptions.

Ok, so I assume the right approach for user space is to do an explicit
DPMS_ON after modesetting.

--Imre

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


Re: [Intel-gfx] [PATCH] drm/i915: force full modeset if the connector is in DPMS OFF mode

2013-05-03 Thread Chris Wilson
On Fri, May 03, 2013 at 02:55:45PM +0300, Imre Deak wrote:
> On Fri, 2013-05-03 at 12:44 +0100, Chris Wilson wrote:
> > On Fri, May 03, 2013 at 02:22:09PM +0300, Imre Deak wrote:
> > > Currently the driver's assumed behavior for a modeset with an attached
> > > FB is that the corresponding connector will be switched to DPMS ON mode
> > > if it happened to be in DPMS OFF (or another power save mode). This
> > > wasn't enforced though if only the FB changed, everything else (format,
> > > connector etc.) remaining the same. In this case we only set the new FB
> > > base and left the connector in the old power save mode.
> > > 
> > > Fix this by forcing a full modeset whenever there is an attached FB and
> > > any affected connector is in a power save mode.
> > > 
> > > Signed-off-by: Imre Deak 
> > 
> > NAK. Papering over bugs, much?
> 
> Ok. I posted this based on our IRC discussion where we seemed to agree
> that DPMS_ON is assumed already; just that it's not done consistently.
> That is if user space does a modeset now that ends up in a full modeset
> (lets say a new FB with different format) then the kernel will do a
> DPMS_ON. But if the new FB happens to be the same format then it won't.
> I thought this is inconsistent and should be fixed.
> 
> Another way to make it consistent would be then to remove DPMS ON from
> the full modeset path..

Right, we have mentioned in the past that the conflation between DPMS
and modesetting is a more recent bug that is going to cause us even more
trouble later. I am not sure how we can fix that as UXA already makes
many bad assumptions.
-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] [TRIVIAL] Fix declaration of intel_gmbus_{is_forced_bit/is_port_falid} in i915 driver.

2013-05-03 Thread Jani Nikula
On Fri, 03 May 2013, Daniel Vetter  wrote:
> On Fri, May 03, 2013 at 11:17:42AM +0200, dl...@gmx.de wrote:
>> From: Jan-Simon Möller 
>> 
>> Description:
>> intel_gmbus_is_forced_bit is no extern as its body is right below.
>> Likewise for intel_gmbus_is_port_valid.
>> 
>> This fixes a compilation issue with clang. An initial version of this patch
>> was developed by PaX Team .
>> This is respin of this patch.
>> 
>> Signed-off-by: Jan-Simon Möller 
>> CC: pagee...@freemail.hu
>> CC: daniel.vet...@ffwll.ch
>> CC: airl...@linux.ie
>> CC: intel-gfx@lists.freedesktop.org
>> CC: dri-de...@lists.freedesktop.org
>> CC: linux-ker...@vger.kernel.org
> Picked up for -fixes, thanks for the patch.

Please drop it.

The patch removes the inline keyword, creating dozens of copies of the
functions, and consequently loads of warnings:

drivers/gpu/drm/i915/i915_drv.h:1803:13: warning: ‘intel_gmbus_is_port_valid’ 
defined but not used [-Wunused-function]
drivers/gpu/drm/i915/i915_drv.h:1812:13: warning: ‘intel_gmbus_is_forced_bit’ 
defined but not used [-Wunused-function]


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


Re: [Intel-gfx] [alsa-devel] [PATCH] drm/i915: Add private api for power well usage -- alignment between graphic team and audio team

2013-05-03 Thread David Henningsson

On 05/03/2013 10:28 AM, Wang, Xingchao wrote:

Hi David,

Thank you very much for your draft patch.
I have some more work on a new patchset, some ideas are from your patch.


Thanks.


Here's a brief introduction of attached patchset:

1. a new bus type in /sound/had_bus.c, used to bind the single module and codec 
device
It looks like ac97_bus.c


I don't understand why this is needed. It does not look like it's used 
from the gfx side either, or anything like that?



2. add a new device node in "struct hda_codec", it's used to register for new 
bus type.

3. a new single module hdmi_i915, which compiled in only when DRM_I915 and 
CODEC_HDMI enabled.
It stores the private API for gfx part.
There's no support to probe haswell hdmi codec only yet. Power well will be 
used only for haswell display audio.

4. power well API implementation in gfx side.

Please feel free to add your idea and I will help test your patch too.


Ok. So the patch I wrote would (if it works) be combined with your patch 
3, which implements the gfx side. The gfx side is not my area of expertise.


The proposed way in my patch would be more elegant since it does not 
introduce any i915 related code in hda_codec* files.


Still, Takashi is the boss here so he has the final say :-)



Thanks
--xingchao


-Original Message-
From: David Henningsson [mailto:david.hennings...@canonical.com]
Sent: Thursday, May 02, 2013 10:49 PM
To: Liam Girdwood
Cc: Barnes, Jesse; alsa-de...@alsa-project.org; Zanoni, Paulo R; Takashi Iwai;
Daniel Vetter; intel-gfx@lists.freedesktop.org; Wysocki, Rafael J; Wang
Xingchao; Wang, Xingchao; Li, Jocelyn; Hindman, Gavin; Daniel Vetter; Lin,
Mengdong; ville.syrj...@linux.intel.com
Subject: Re: [alsa-devel] [PATCH] drm/i915: Add private api for power well
usage -- alignment between graphic team and audio team

On 04/30/2013 04:41 PM, Liam Girdwood wrote:

On Tue, 2013-04-30 at 12:29 +0200, David Henningsson wrote:

On 04/29/2013 05:02 PM, Jesse Barnes wrote:

On Sat, 27 Apr 2013 13:35:29 +0200
Daniel Vetter  wrote:


On Sat, Apr 27, 2013 at 09:20:39AM +, Wang, Xingchao wrote:

Let me throw a basic proposal on Audio driver side,  please give your

comments freely.


it contains the power well control usage points:
#1: audio request power well at boot up.
I915 may shut down power well after bootup initialization, as there's no

monitor connected outside or only eDP on pipe A.

#2: audio request power on resume
After exit from D3 mode, audio driver quest power on. This may happen

at normal resume or runtime resume.

#3: audio release power well control at suspend Audio driver will
let i915 know it doensot need power well anymore as it's going to

suspend. This may happened at normal suspend or runtime suspend point.

#4: audio release power well when module unload Audio release
power well at remove callback to let i915 know.


I miss the power well grab/dropping at runtime from the audio side.
If the audio driver forces the power well to be on the entire time
it's loaded, that's not good, since the power well stuff is very much for

runtime PM.

We _must_ be able to switch off the power well whenever possible.


Xingchao, I'm not an audio developer so I'm probably way off.

But what we really need is a very small and targeted set of calls
into the i915 driver from say the HDMI driver in HDA.  It looks like
the prepare/cleanup pair in the pcm_ops structure might be the right
place to put things?  If that's too fine grained, you could do it at
open/close time I guess, but the danger there is that some app will
keep the device open even while it's not playing.

If that won't work, maybe calling i915 from hda_power_work in the
higher level code would be better?

For detecting whether to call i915 at all, you can filter on the PCI
IDs (just look for an Intel graphics device and if present, try to
get the i915 symbols for the power functions).

--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -3860,6 +3860,8 @@ static unsigned int

hda_call_codec_suspend(struct hda_code

   codec->patch_ops.suspend(codec);
   hda_cleanup_all_streams(codec);
   state = hda_set_power_state(codec, AC_PWRST_D3);
+   if (i915_shared_power_well)
+   i915_put_power_well(codec->i915_data);
   /* Cancel delayed work if we aren't currently running from it.

*/

   if (!in_wq)
   cancel_delayed_work_sync(&codec->power_work);
@@ -4807,6 +4809,9 @@ static void __snd_hda_power_up(struct

hda_codec *codec, bo

   return;
   spin_unlock(&codec->power_lock);

+   if (i915_shared_power_well)
+   i915_get_power_well(codec->i915_data);


Is it wise that a _get function actually has side effects? Perhaps
_push and _pop or something else would be better semantics.



I think the intention here is to model on the PM runtime subsystem
where we can get/put the reference count on a PM resource. I'd expect
with this A

Re: [Intel-gfx] [PATCH] drm/i915: force full modeset if the connector is in DPMS OFF mode

2013-05-03 Thread Imre Deak
On Fri, 2013-05-03 at 12:44 +0100, Chris Wilson wrote:
> On Fri, May 03, 2013 at 02:22:09PM +0300, Imre Deak wrote:
> > Currently the driver's assumed behavior for a modeset with an attached
> > FB is that the corresponding connector will be switched to DPMS ON mode
> > if it happened to be in DPMS OFF (or another power save mode). This
> > wasn't enforced though if only the FB changed, everything else (format,
> > connector etc.) remaining the same. In this case we only set the new FB
> > base and left the connector in the old power save mode.
> > 
> > Fix this by forcing a full modeset whenever there is an attached FB and
> > any affected connector is in a power save mode.
> > 
> > Signed-off-by: Imre Deak 
> 
> NAK. Papering over bugs, much?

Ok. I posted this based on our IRC discussion where we seemed to agree
that DPMS_ON is assumed already; just that it's not done consistently.
That is if user space does a modeset now that ends up in a full modeset
(lets say a new FB with different format) then the kernel will do a
DPMS_ON. But if the new FB happens to be the same format then it won't.
I thought this is inconsistent and should be fixed.

Another way to make it consistent would be then to remove DPMS ON from
the full modeset path..

--Imre


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


Re: [Intel-gfx] [PATCH] drm/i915: force full modeset if the connector is in DPMS OFF mode

2013-05-03 Thread Jani Nikula
On Fri, 03 May 2013, Imre Deak  wrote:
> Currently the driver's assumed behavior for a modeset with an attached
> FB is that the corresponding connector will be switched to DPMS ON mode
> if it happened to be in DPMS OFF (or another power save mode). This
> wasn't enforced though if only the FB changed, everything else (format,
> connector etc.) remaining the same. In this case we only set the new FB
> base and left the connector in the old power save mode.
>
> Fix this by forcing a full modeset whenever there is an attached FB and
> any affected connector is in a power save mode.
>
> Signed-off-by: Imre Deak 
> ---
>  drivers/gpu/drm/i915/intel_display.c |   21 -
>  1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 2939524..0aee2ba 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8380,6 +8380,21 @@ static void intel_set_config_restore_state(struct 
> drm_device *dev,
>   }
>  }
>  
> +static bool
> +crtc_connector_off(struct drm_crtc *crtc, struct drm_connector *connectors,
> +int num_connectors)
> +{
> + int i;
> +
> + for (i = 0; i < num_connectors; i++)
> + if (connectors[i].encoder &&
> + connectors[i].encoder->crtc == crtc &&
> + connectors[i].dpms != DRM_MODE_DPMS_ON)
> + return true;
> +
> + return false;
> +}
> +
>  static void
>  intel_set_config_compute_mode_changes(struct drm_mode_set *set,
> struct intel_set_config *config)
> @@ -8397,8 +8412,12 @@ intel_set_config_compute_mode_changes(struct 
> drm_mode_set *set,
>   } else if (set->fb->pixel_format !=
>  set->crtc->fb->pixel_format) {
>   config->mode_changed = true;
> - } else
> + } else if (crtc_connector_off(set->crtc, *set->connectors,
> +   set->num_connectors)) {

Bikeshed, crtc_connector_off() sounds like disabling something, while
is_crtc_connector_off() would sound like checking the state.

Jani.


> + config->mode_changed = true;
> + } else {
>   config->fb_changed = true;
> + }
>   }
>  
>   if (set->fb && (set->x != set->crtc->x || set->y != set->crtc->y))
> -- 
> 1.7.10.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: force full modeset if the connector is in DPMS OFF mode

2013-05-03 Thread Chris Wilson
On Fri, May 03, 2013 at 02:22:09PM +0300, Imre Deak wrote:
> Currently the driver's assumed behavior for a modeset with an attached
> FB is that the corresponding connector will be switched to DPMS ON mode
> if it happened to be in DPMS OFF (or another power save mode). This
> wasn't enforced though if only the FB changed, everything else (format,
> connector etc.) remaining the same. In this case we only set the new FB
> base and left the connector in the old power save mode.
> 
> Fix this by forcing a full modeset whenever there is an attached FB and
> any affected connector is in a power save mode.
> 
> Signed-off-by: Imre Deak 

NAK. Papering over bugs, much?
-Chris

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


Re: [Intel-gfx] [PATCH] drm/i915: force full modeset if the connector is in DPMS OFF mode

2013-05-03 Thread Imre Deak
On Fri, 2013-05-03 at 13:32 +0200, Daniel Vetter wrote:
> On Fri, May 3, 2013 at 1:27 PM, Daniel Vetter  wrote:
> > On Fri, May 3, 2013 at 1:22 PM, Imre Deak  wrote:
> >> Currently the driver's assumed behavior for a modeset with an attached
> >> FB is that the corresponding connector will be switched to DPMS ON mode
> >> if it happened to be in DPMS OFF (or another power save mode). This
> >> wasn't enforced though if only the FB changed, everything else (format,
> >> connector etc.) remaining the same. In this case we only set the new FB
> >> base and left the connector in the old power save mode.
> >>
> >> Fix this by forcing a full modeset whenever there is an attached FB and
> >> any affected connector is in a power save mode.
> >>
> >> Signed-off-by: Imre Deak 
> >
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61642

Chris marked this as a dup for another bug with an other root cause.

> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59834

Yes, this is tested against the patch already.

> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59339

Ok I'll point them to the fix.

> > Safe for the first just a few wild guesses. Can you please spam
> > bugzilla with this patch (there's tons more spurious kms_flip fail
> > iirc, also a few of the kms_flip jitter reports should be fixed by
> > now).
> 
> Also, and i-g-t to specifically exercise this would be nice ... Maybe
> easiest on top of kms_flip, using dpms off, but then a set_base as an
> implicit dpms on.

Ok, will put together something.

--Imre

> -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: force full modeset if the connector is in DPMS OFF mode

2013-05-03 Thread Daniel Vetter
On Fri, May 3, 2013 at 1:27 PM, Daniel Vetter  wrote:
> On Fri, May 3, 2013 at 1:22 PM, Imre Deak  wrote:
>> Currently the driver's assumed behavior for a modeset with an attached
>> FB is that the corresponding connector will be switched to DPMS ON mode
>> if it happened to be in DPMS OFF (or another power save mode). This
>> wasn't enforced though if only the FB changed, everything else (format,
>> connector etc.) remaining the same. In this case we only set the new FB
>> base and left the connector in the old power save mode.
>>
>> Fix this by forcing a full modeset whenever there is an attached FB and
>> any affected connector is in a power save mode.
>>
>> Signed-off-by: Imre Deak 
>
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61642
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59834
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59339
>
> Safe for the first just a few wild guesses. Can you please spam
> bugzilla with this patch (there's tons more spurious kms_flip fail
> iirc, also a few of the kms_flip jitter reports should be fixed by
> now).

Also, and i-g-t to specifically exercise this would be nice ... Maybe
easiest on top of kms_flip, using dpms off, but then a set_base as an
implicit dpms on.
-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: force full modeset if the connector is in DPMS OFF mode

2013-05-03 Thread Daniel Vetter
On Fri, May 3, 2013 at 1:22 PM, Imre Deak  wrote:
> Currently the driver's assumed behavior for a modeset with an attached
> FB is that the corresponding connector will be switched to DPMS ON mode
> if it happened to be in DPMS OFF (or another power save mode). This
> wasn't enforced though if only the FB changed, everything else (format,
> connector etc.) remaining the same. In this case we only set the new FB
> base and left the connector in the old power save mode.
>
> Fix this by forcing a full modeset whenever there is an attached FB and
> any affected connector is in a power save mode.
>
> Signed-off-by: Imre Deak 


Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61642
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59834
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59339

Safe for the first just a few wild guesses. Can you please spam
bugzilla with this patch (there's tons more spurious kms_flip fail
iirc, also a few of the kms_flip jitter reports should be fixed by
now).

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 6/6] drm/i915: fix up adjusted_mode tracking for interlaced modes

2013-05-03 Thread Daniel Vetter
On Fri, May 3, 2013 at 11:49 AM, Daniel Vetter  wrote:
> With the hw state readout&check code it's important that the values we
> keep around are the canonical ones. Unfortunately when adding the pipe
> timings readout support I've missed that the write side adjusts the
> timings in the pipe config.
>
> Fix this up.
>
> Reported-by: Paulo Zanoni 
> Signed-off-by: Daniel Vetter 

I better be a good example ;-) So:

This fixes a regression introduced in

commit 1bd1bd806037af04dd1d7bdd39b2b04090c10d2c
Author: Daniel Vetter 
Date:   Mon Apr 29 21:56:12 2013 +0200

drm/i915: hw state readout support for pipe timings

Cc: Mika Kuoppala 

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


Re: [Intel-gfx] [PATCH] drm/i915: fix Haswell pfit power well check v2

2013-05-03 Thread Daniel Vetter
On Fri, May 3, 2013 at 12:30 AM, Jesse Barnes  wrote:
> We can't read the pfit regs if the power well is off, so use the cached
> value.
>
> v2: re-add lost comment (Jesse)
> make sure the crtc using the fitter is actually enabled (Jesse)
>
> Signed-off-by: Jesse Barnes 

Just a quick maintainer bikeshed: For fixups I highly prefer a quick
git commit citation of the regressing commit plus all the cc+reviewers
of the original patch in the cc section of this one. Just so that
reviewers don't miss a good chance to learn something.
-Daniel

> ---
>  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 6504337..6be34f2 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5918,7 +5918,7 @@ static void haswell_modeset_global_resources(struct 
> drm_device *dev)
>  * sequence that's not yet available. Just in case desktop eDP
>  * on PORT D is possible on haswell, too. */
> /* Even the eDP panel fitter is outside the always-on well. */
> -   if (I915_READ(PF_WIN_SZ(crtc->pipe)))
> +   if (crtc->config.pch_pfit.size && crtc->base.enabled)
> enable = true;
> }
>
> --
> 1.7.10.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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


[Intel-gfx] [PATCH] drm/i915: force full modeset if the connector is in DPMS OFF mode

2013-05-03 Thread Imre Deak
Currently the driver's assumed behavior for a modeset with an attached
FB is that the corresponding connector will be switched to DPMS ON mode
if it happened to be in DPMS OFF (or another power save mode). This
wasn't enforced though if only the FB changed, everything else (format,
connector etc.) remaining the same. In this case we only set the new FB
base and left the connector in the old power save mode.

Fix this by forcing a full modeset whenever there is an attached FB and
any affected connector is in a power save mode.

Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/intel_display.c |   21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 2939524..0aee2ba 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8380,6 +8380,21 @@ static void intel_set_config_restore_state(struct 
drm_device *dev,
}
 }
 
+static bool
+crtc_connector_off(struct drm_crtc *crtc, struct drm_connector *connectors,
+  int num_connectors)
+{
+   int i;
+
+   for (i = 0; i < num_connectors; i++)
+   if (connectors[i].encoder &&
+   connectors[i].encoder->crtc == crtc &&
+   connectors[i].dpms != DRM_MODE_DPMS_ON)
+   return true;
+
+   return false;
+}
+
 static void
 intel_set_config_compute_mode_changes(struct drm_mode_set *set,
  struct intel_set_config *config)
@@ -8397,8 +8412,12 @@ intel_set_config_compute_mode_changes(struct 
drm_mode_set *set,
} else if (set->fb->pixel_format !=
   set->crtc->fb->pixel_format) {
config->mode_changed = true;
-   } else
+   } else if (crtc_connector_off(set->crtc, *set->connectors,
+ set->num_connectors)) {
+   config->mode_changed = true;
+   } else {
config->fb_changed = true;
+   }
}
 
if (set->fb && (set->x != set->crtc->x || set->y != set->crtc->y))
-- 
1.7.10.4

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


[Intel-gfx] [PATCH v3] drm/i915: hsw: fix link training for eDP on port-A

2013-05-03 Thread Imre Deak
According to BSpec the link training sequence for eDP on HSW port-A
should be as follows:

1. link training: clock recovery
2. link training: equalization
3. link training: set idle transmission mode
4. display pipe enable
5. link training: disable (set normal mode)

Contrary to this at the moment we don't do step 3. and we do step 5.
before step 4. Fix this by setting idle transmission mode for eDP at
the end of intel_dp_complete_link_train and adding a new
intel_dp_stop_link_training function to disable link training. With
these changes we'll end up with the following functions corresponding
to the above steps:

intel_dp_start_link_train-> step 1.
intel_dp_complete_link_train -> step 2., step 3.
intel_dp_stop_link_train -> step 5.

For port-A we'll call intel_dp_stop_link_train only after enabling the
pipe, for everything else we'll call it right after
intel_dp_complete_link_train to preserve the current behavior.

Tested on HSW/HSW-ULT.

In v2:
- Due to a HW issue we must set idle transmission mode for port-A too
  before enabling the pipe. Thanks for Arthur Runyan for explaining
  this.
- Update the patch subject to make it clear that it's an eDP fix, DP is
  not affected.

v3:
- rename intel_dp_link_train() to intel_dp_set_link_train(), use 'val'
  instead 'l' as var name. (Paulo)

Signed-off-by: Imre Deak 
Reviewed-by: Paulo Zanoni 
Tested-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/intel_ddi.c |5 
 drivers/gpu/drm/i915/intel_dp.c  |   59 --
 drivers/gpu/drm/i915/intel_drv.h |1 +
 3 files changed, 50 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 0d7cf31..f3d0f21 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1265,6 +1265,8 @@ static void intel_ddi_pre_enable(struct intel_encoder 
*intel_encoder)
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
intel_dp_start_link_train(intel_dp);
intel_dp_complete_link_train(intel_dp);
+   if (port != PORT_A)
+   intel_dp_stop_link_train(intel_dp);
}
 }
 
@@ -1326,6 +1328,9 @@ static void intel_enable_ddi(struct intel_encoder 
*intel_encoder)
} else if (type == INTEL_OUTPUT_EDP) {
struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
+   if (port == PORT_A)
+   intel_dp_stop_link_train(intel_dp);
+
ironlake_edp_backlight_on(intel_dp);
}
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 13d2fd6..e991600 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1437,6 +1437,7 @@ static void intel_enable_dp(struct intel_encoder *encoder)
ironlake_edp_panel_on(intel_dp);
ironlake_edp_panel_vdd_off(intel_dp, true);
intel_dp_complete_link_train(intel_dp);
+   intel_dp_stop_link_train(intel_dp);
ironlake_edp_backlight_on(intel_dp);
 
if (IS_VALLEYVIEW(dev)) {
@@ -1935,10 +1936,9 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
struct drm_i915_private *dev_priv = dev->dev_private;
enum port port = intel_dig_port->port;
int ret;
-   uint32_t temp;
 
if (HAS_DDI(dev)) {
-   temp = I915_READ(DP_TP_CTL(port));
+   uint32_t temp = I915_READ(DP_TP_CTL(port));
 
if (dp_train_pat & DP_LINK_SCRAMBLING_DISABLE)
temp |= DP_TP_CTL_SCRAMBLE_DISABLE;
@@ -1948,18 +1948,6 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
temp &= ~DP_TP_CTL_LINK_TRAIN_MASK;
switch (dp_train_pat & DP_TRAINING_PATTERN_MASK) {
case DP_TRAINING_PATTERN_DISABLE:
-
-   if (port != PORT_A) {
-   temp |= DP_TP_CTL_LINK_TRAIN_IDLE;
-   I915_WRITE(DP_TP_CTL(port), temp);
-
-   if (wait_for((I915_READ(DP_TP_STATUS(port)) &
- DP_TP_STATUS_IDLE_DONE), 1))
-   DRM_ERROR("Timed out waiting for DP 
idle patterns\n");
-
-   temp &= ~DP_TP_CTL_LINK_TRAIN_MASK;
-   }
-
temp |= DP_TP_CTL_LINK_TRAIN_NORMAL;
 
break;
@@ -2035,6 +2023,37 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
return true;
 }
 
+static void intel_dp_set_idle_link_train(struct intel_dp *intel_dp)
+{
+   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+   struct drm_device *dev = intel_dig_port->base.base.dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   enum port port = intel_dig_port->port;
+   uint32_t val;
+
+   if (!HAS_DDI(dev))
+   return;
+
+   val = I915_READ(DP_TP_CTL(port));
+   val

[Intel-gfx] [PATCH 6/6] drm/i915: fix up adjusted_mode tracking for interlaced modes

2013-05-03 Thread Daniel Vetter
With the hw state readout&check code it's important that the values we
keep around are the canonical ones. Unfortunately when adding the pipe
timings readout support I've missed that the write side adjusts the
timings in the pipe config.

Fix this up.

Reported-by: Paulo Zanoni 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_display.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 9ae60c2..3b7fae1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4729,12 +4729,17 @@ static void intel_set_pipe_timings(struct intel_crtc 
*intel_crtc,
struct drm_i915_private *dev_priv = dev->dev_private;
enum pipe pipe = intel_crtc->pipe;
enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
-   uint32_t vsyncshift;
+   uint32_t vsyncshift, crtc_vtotal, crtc_vblank_end;
+
+   /* We need to be careful not to changed the adjusted mode, for otherwise
+* the hw state checker will get angry at the mismatch. */
+   crtc_vtotal = adjusted_mode->crtc_vtotal;
+   crtc_vblank_end = adjusted_mode->crtc_vblank_end;
 
if (!IS_GEN2(dev) && adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
/* the chip adds 2 halflines automatically */
-   adjusted_mode->crtc_vtotal -= 1;
-   adjusted_mode->crtc_vblank_end -= 1;
+   crtc_vtotal -= 1;
+   crtc_vblank_end -= 1;
vsyncshift = adjusted_mode->crtc_hsync_start
 - adjusted_mode->crtc_htotal / 2;
} else {
@@ -4756,10 +4761,10 @@ static void intel_set_pipe_timings(struct intel_crtc 
*intel_crtc,
 
I915_WRITE(VTOTAL(cpu_transcoder),
   (adjusted_mode->crtc_vdisplay - 1) |
-  ((adjusted_mode->crtc_vtotal - 1) << 16));
+  ((crtc_vtotal - 1) << 16));
I915_WRITE(VBLANK(cpu_transcoder),
   (adjusted_mode->crtc_vblank_start - 1) |
-  ((adjusted_mode->crtc_vblank_end - 1) << 16));
+  ((crtc_vblank_end - 1) << 16));
I915_WRITE(VSYNC(cpu_transcoder),
   (adjusted_mode->crtc_vsync_start - 1) |
   ((adjusted_mode->crtc_vsync_end - 1) << 16));
-- 
1.8.1.4

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


[Intel-gfx] [PATCH 5/6] drm/i915: make intel_cpt_verify_modeset static

2013-05-03 Thread Daniel Vetter
Only one caller. Also drop the intel_ prefix as is now customary for
platform specific and static functions.

Reviewed-by: Paulo Zanoni 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_display.c | 4 ++--
 drivers/gpu/drm/i915/intel_drv.h | 1 -
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index aa4de35..9ae60c2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3229,7 +3229,7 @@ prepare: /* separate function? */
return pll;
 }
 
-void intel_cpt_verify_modeset(struct drm_device *dev, int pipe)
+static void cpt_verify_modeset(struct drm_device *dev, int pipe)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
int dslreg = PIPEDSL(pipe);
@@ -3334,7 +3334,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
encoder->enable(encoder);
 
if (HAS_PCH_CPT(dev))
-   intel_cpt_verify_modeset(dev, intel_crtc->pipe);
+   cpt_verify_modeset(dev, intel_crtc->pipe);
 
/*
 * There seems to be a race in PCH platform hw (at least on some
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 88890a3..e3108c0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -718,7 +718,6 @@ extern void assert_pipe(struct drm_i915_private *dev_priv, 
enum pipe pipe,
 extern void intel_init_clock_gating(struct drm_device *dev);
 extern void intel_write_eld(struct drm_encoder *encoder,
struct drm_display_mode *mode);
-extern void intel_cpt_verify_modeset(struct drm_device *dev, int pipe);
 extern void intel_prepare_ddi(struct drm_device *dev);
 extern void hsw_fdi_link_train(struct drm_crtc *crtc);
 extern void intel_ddi_init(struct drm_device *dev, enum port port);
-- 
1.8.1.4

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


[Intel-gfx] [PATCH 4/6] drm/i915: Apply OCD to data/link m/n register #defines

2013-05-03 Thread Daniel Vetter
- PCH_ prefix for pch registers on ibx/cpt/ppt.
- Drop the DP_ from the link defines, redundant.
- Drop the GMCH from the data defines and instead give the special g4x
  registers a consistent _G4X postfix.

v2:
- Realign #defines and use tabs (Paulo).

Reviewed-by: Paulo Zanoni 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_reg.h  | 74 ++--
 drivers/gpu/drm/i915/i915_ums.c  | 32 
 drivers/gpu/drm/i915/intel_display.c | 16 
 3 files changed, 61 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 66c906d..dca3504 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2768,8 +2768,8 @@
  * which is after the LUTs, so we want the bytes for our color format.
  * For our current usage, this is always 3, one byte for R, G and B.
  */
-#define _PIPEA_GMCH_DATA_M 0x70050
-#define _PIPEB_GMCH_DATA_M 0x71050
+#define _PIPEA_DATA_M_G4X  0x70050
+#define _PIPEB_DATA_M_G4X  0x71050
 
 /* Transfer unit size for display port - 1, default is 0x3f (for TU size 64) */
 #define  TU_SIZE(x) (((x)-1) << 25) /* default size 64 */
@@ -2779,8 +2779,8 @@
 #define  DATA_LINK_M_N_MASK(0xff)
 #define  DATA_LINK_N_MAX   (0x80)
 
-#define _PIPEA_GMCH_DATA_N 0x70054
-#define _PIPEB_GMCH_DATA_N 0x71054
+#define _PIPEA_DATA_N_G4X  0x70054
+#define _PIPEB_DATA_N_G4X  0x71054
 
 /*
  * Computing Link M and N values for the Display Port link
@@ -2793,16 +2793,16 @@
  * Attributes and VB-ID.
  */
 
-#define _PIPEA_DP_LINK_M   0x70060
-#define _PIPEB_DP_LINK_M   0x71060
+#define _PIPEA_LINK_M_G4X  0x70060
+#define _PIPEB_LINK_M_G4X  0x71060
 
-#define _PIPEA_DP_LINK_N   0x70064
-#define _PIPEB_DP_LINK_N   0x71064
+#define _PIPEA_LINK_N_G4X  0x70064
+#define _PIPEB_LINK_N_G4X  0x71064
 
-#define PIPE_GMCH_DATA_M(pipe) _PIPE(pipe, _PIPEA_GMCH_DATA_M, 
_PIPEB_GMCH_DATA_M)
-#define PIPE_GMCH_DATA_N(pipe) _PIPE(pipe, _PIPEA_GMCH_DATA_N, 
_PIPEB_GMCH_DATA_N)
-#define PIPE_DP_LINK_M(pipe) _PIPE(pipe, _PIPEA_DP_LINK_M, _PIPEB_DP_LINK_M)
-#define PIPE_DP_LINK_N(pipe) _PIPE(pipe, _PIPEA_DP_LINK_N, _PIPEB_DP_LINK_N)
+#define PIPE_DATA_M_G4X(pipe) _PIPE(pipe, _PIPEA_DATA_M_G4X, _PIPEB_DATA_M_G4X)
+#define PIPE_DATA_N_G4X(pipe) _PIPE(pipe, _PIPEA_DATA_N_G4X, _PIPEB_DATA_N_G4X)
+#define PIPE_LINK_M_G4X(pipe) _PIPE(pipe, _PIPEA_LINK_M_G4X, _PIPEB_LINK_M_G4X)
+#define PIPE_LINK_N_G4X(pipe) _PIPE(pipe, _PIPEA_LINK_N_G4X, _PIPEB_LINK_N_G4X)
 
 /* Display & cursor control */
 
@@ -3945,14 +3945,14 @@
 #define  TRANS_VSYNC_START_SHIFT   0
 #define _PCH_TRANS_VSYNCSHIFT_A0xe0028
 
-#define _TRANSA_DATA_M1  0xe0030
-#define _TRANSA_DATA_N1  0xe0034
-#define _TRANSA_DATA_M2  0xe0038
-#define _TRANSA_DATA_N2  0xe003c
-#define _TRANSA_DP_LINK_M1   0xe0040
-#define _TRANSA_DP_LINK_N1   0xe0044
-#define _TRANSA_DP_LINK_M2   0xe0048
-#define _TRANSA_DP_LINK_N2   0xe004c
+#define _PCH_TRANSA_DATA_M10xe0030
+#define _PCH_TRANSA_DATA_N10xe0034
+#define _PCH_TRANSA_DATA_M20xe0038
+#define _PCH_TRANSA_DATA_N20xe003c
+#define _PCH_TRANSA_LINK_M10xe0040
+#define _PCH_TRANSA_LINK_N10xe0044
+#define _PCH_TRANSA_LINK_M20xe0048
+#define _PCH_TRANSA_LINK_N20xe004c
 
 /* Per-transcoder DIP controls */
 
@@ -4038,23 +4038,23 @@
 #define PCH_TRANS_VSYNCSHIFT(pipe) _PIPE(pipe, _PCH_TRANS_VSYNCSHIFT_A, \
 _PCH_TRANS_VSYNCSHIFT_B)
 
-#define _TRANSB_DATA_M1  0xe1030
-#define _TRANSB_DATA_N1  0xe1034
-#define _TRANSB_DATA_M2  0xe1038
-#define _TRANSB_DATA_N2  0xe103c
-#define _TRANSB_DP_LINK_M1   0xe1040
-#define _TRANSB_DP_LINK_N1   0xe1044
-#define _TRANSB_DP_LINK_M2   0xe1048
-#define _TRANSB_DP_LINK_N2   0xe104c
-
-#define TRANSDATA_M1(pipe) _PIPE(pipe, _TRANSA_DATA_M1, _TRANSB_DATA_M1)
-#define TRANSDATA_N1(pipe) _PIPE(pipe, _TRANSA_DATA_N1, _TRANSB_DATA_N1)
-#define TRANSDATA_M2(pipe) _PIPE(pipe, _TRANSA_DATA_M2, _TRANSB_DATA_M2)
-#define TRANSDATA_N2(pipe) _PIPE(pipe, _TRANSA_DATA_N2, _TRANSB_DATA_N2)
-#define TRANSDPLINK_M1(pipe) _PIPE(pipe, _TRANSA_DP_LINK_M1, 
_TRANSB_DP_LINK_M1)
-#define TRANSDPLINK_N1(pipe) _PIPE(pipe, _TRANSA_DP_LINK_N1, 
_TRANSB_DP_LINK_N1)
-#define TRANSDPLINK_M2(pipe) _PIPE(pipe, _TRANSA_DP_LINK_M2, 
_TRANSB_DP_LINK_M2)
-#define TRANSDPLINK_N2(pipe) _PIPE(pipe, _TRANSA_DP_LINK_N2, 
_TRANSB_DP_LINK_N2)
+#define _PCH_TRANSB_DATA_M10xe1030
+#define _PCH_TRANSB_DATA_N10xe1034
+#define _PCH_TRANSB_DATA_M20xe1038
+#define _PCH_TRANSB_DATA_N20xe103c
+#define _PCH_TRANSB_LINK_M10xe1040
+#define _PCH_TRANSB_LINK_N10xe1044
+#define _PCH_TR

[Intel-gfx] [PATCH 3/6] drm/i915: make set_m_n functions static

2013-05-03 Thread Daniel Vetter
This is possible thanks to moving the m/n stuff into pipe_config.

Unfortunately we need to move them a bit to avoid forward
declarations.

Reviewed-by: Paulo Zanoni 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_display.c | 68 ++--
 drivers/gpu/drm/i915/intel_drv.h |  4 ---
 2 files changed, 34 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 9c85270..3dd1059 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4392,6 +4392,40 @@ static void vlv_pllb_recal_opamp(struct drm_i915_private 
*dev_priv)
intel_dpio_write(dev_priv, DPIO_CALIBRATION, reg_val);
 }
 
+static void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
+struct intel_link_m_n *m_n)
+{
+   struct drm_device *dev = crtc->base.dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   int pipe = crtc->pipe;
+
+   I915_WRITE(TRANSDATA_M1(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
+   I915_WRITE(TRANSDATA_N1(pipe), m_n->gmch_n);
+   I915_WRITE(TRANSDPLINK_M1(pipe), m_n->link_m);
+   I915_WRITE(TRANSDPLINK_N1(pipe), m_n->link_n);
+}
+
+static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
+struct intel_link_m_n *m_n)
+{
+   struct drm_device *dev = crtc->base.dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   int pipe = crtc->pipe;
+   enum transcoder transcoder = crtc->config.cpu_transcoder;
+
+   if (INTEL_INFO(dev)->gen >= 5) {
+   I915_WRITE(PIPE_DATA_M1(transcoder), TU_SIZE(m_n->tu) | 
m_n->gmch_m);
+   I915_WRITE(PIPE_DATA_N1(transcoder), m_n->gmch_n);
+   I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
+   I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
+   } else {
+   I915_WRITE(PIPE_GMCH_DATA_M(pipe), TU_SIZE(m_n->tu) | 
m_n->gmch_m);
+   I915_WRITE(PIPE_GMCH_DATA_N(pipe), m_n->gmch_n);
+   I915_WRITE(PIPE_DP_LINK_M(pipe), m_n->link_m);
+   I915_WRITE(PIPE_DP_LINK_N(pipe), m_n->link_n);
+   }
+}
+
 static void intel_dp_set_m_n(struct intel_crtc *crtc)
 {
if (crtc->config.has_pch_encoder)
@@ -5611,40 +5645,6 @@ int ironlake_get_lanes_required(int target_clock, int 
link_bw, int bpp)
return bps / (link_bw * 8) + 1;
 }
 
-void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
- struct intel_link_m_n *m_n)
-{
-   struct drm_device *dev = crtc->base.dev;
-   struct drm_i915_private *dev_priv = dev->dev_private;
-   int pipe = crtc->pipe;
-
-   I915_WRITE(TRANSDATA_M1(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
-   I915_WRITE(TRANSDATA_N1(pipe), m_n->gmch_n);
-   I915_WRITE(TRANSDPLINK_M1(pipe), m_n->link_m);
-   I915_WRITE(TRANSDPLINK_N1(pipe), m_n->link_n);
-}
-
-void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
- struct intel_link_m_n *m_n)
-{
-   struct drm_device *dev = crtc->base.dev;
-   struct drm_i915_private *dev_priv = dev->dev_private;
-   int pipe = crtc->pipe;
-   enum transcoder transcoder = crtc->config.cpu_transcoder;
-
-   if (INTEL_INFO(dev)->gen >= 5) {
-   I915_WRITE(PIPE_DATA_M1(transcoder), TU_SIZE(m_n->tu) | 
m_n->gmch_m);
-   I915_WRITE(PIPE_DATA_N1(transcoder), m_n->gmch_n);
-   I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
-   I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
-   } else {
-   I915_WRITE(PIPE_GMCH_DATA_M(pipe), TU_SIZE(m_n->tu) | 
m_n->gmch_m);
-   I915_WRITE(PIPE_GMCH_DATA_N(pipe), m_n->gmch_n);
-   I915_WRITE(PIPE_DP_LINK_M(pipe), m_n->link_m);
-   I915_WRITE(PIPE_DP_LINK_N(pipe), m_n->link_n);
-   }
-}
-
 static bool ironlake_needs_fb_cb_tune(struct dpll *dpll, int factor)
 {
return i9xx_dpll_compute_m(dpll) < factor * dpll->n;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index dfcf546..88890a3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -719,10 +719,6 @@ extern void intel_init_clock_gating(struct drm_device 
*dev);
 extern void intel_write_eld(struct drm_encoder *encoder,
struct drm_display_mode *mode);
 extern void intel_cpt_verify_modeset(struct drm_device *dev, int pipe);
-extern void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
-struct intel_link_m_n *m_n);
-extern void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
-struct intel_link_m_n *m_n);
 extern void intel_prepare_ddi(struct drm_device *dev);
 extern void hsw_fdi_link_train(struct drm_crtc *crtc);
 extern void intel_ddi_init(struct drm

[Intel-gfx] [PATCH 2/6] drm/i915: PCH_ prefix for transcoder timings

2013-05-03 Thread Daniel Vetter
While at it, also extract a common helper to copy the timings from the
cpu transcoder to the pch transcoder. That way it's really explicit
how the lpt transcoder is hardcoded.

v2:
- Re-align #defines properly (Paulo).
- Use cpu_transcoder when copying pipe timings (Paulo).
- s/intel_pch_transcoder_enable/intel_pch_transcoder_set_timings/
  since we already have a pch transcoder enable function, and this is
  clearer, too.
- Fixup 80 char line overflow in intel_display.c. I've opted to ignore
  this in i915_reg.h and i915_ums.c since meh.

Cc: Paulo Zanoni 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_reg.h  | 70 ++--
 drivers/gpu/drm/i915/i915_ums.c  | 48 -
 drivers/gpu/drm/i915/intel_display.c | 42 +-
 3 files changed, 85 insertions(+), 75 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d6fc8e7..66c906d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3925,25 +3925,25 @@
 
 /* transcoder */
 
-#define _TRANS_HTOTAL_A  0xe
-#define  TRANS_HTOTAL_SHIFT 16
-#define  TRANS_HACTIVE_SHIFT0
-#define _TRANS_HBLANK_A  0xe0004
-#define  TRANS_HBLANK_END_SHIFT 16
-#define  TRANS_HBLANK_START_SHIFT 0
-#define _TRANS_HSYNC_A   0xe0008
-#define  TRANS_HSYNC_END_SHIFT  16
-#define  TRANS_HSYNC_START_SHIFT 0
-#define _TRANS_VTOTAL_A  0xe000c
-#define  TRANS_VTOTAL_SHIFT 16
-#define  TRANS_VACTIVE_SHIFT0
-#define _TRANS_VBLANK_A  0xe0010
-#define  TRANS_VBLANK_END_SHIFT 16
-#define  TRANS_VBLANK_START_SHIFT 0
-#define _TRANS_VSYNC_A   0xe0014
-#define  TRANS_VSYNC_END_SHIFT  16
-#define  TRANS_VSYNC_START_SHIFT 0
-#define _TRANS_VSYNCSHIFT_A0xe0028
+#define _PCH_TRANS_HTOTAL_A0xe
+#define  TRANS_HTOTAL_SHIFT16
+#define  TRANS_HACTIVE_SHIFT   0
+#define _PCH_TRANS_HBLANK_A0xe0004
+#define  TRANS_HBLANK_END_SHIFT16
+#define  TRANS_HBLANK_START_SHIFT  0
+#define _PCH_TRANS_HSYNC_A 0xe0008
+#define  TRANS_HSYNC_END_SHIFT 16
+#define  TRANS_HSYNC_START_SHIFT   0
+#define _PCH_TRANS_VTOTAL_A0xe000c
+#define  TRANS_VTOTAL_SHIFT16
+#define  TRANS_VACTIVE_SHIFT   0
+#define _PCH_TRANS_VBLANK_A0xe0010
+#define  TRANS_VBLANK_END_SHIFT16
+#define  TRANS_VBLANK_START_SHIFT  0
+#define _PCH_TRANS_VSYNC_A 0xe0014
+#define  TRANS_VSYNC_END_SHIFT 16
+#define  TRANS_VSYNC_START_SHIFT   0
+#define _PCH_TRANS_VSYNCSHIFT_A0xe0028
 
 #define _TRANSA_DATA_M1  0xe0030
 #define _TRANSA_DATA_N1  0xe0034
@@ -4021,22 +4021,22 @@
 #define HSW_TVIDEO_DIP_VSC_DATA(trans) \
 _TRANSCODER(trans, HSW_VIDEO_DIP_VSC_DATA_A, HSW_VIDEO_DIP_VSC_DATA_B)
 
-#define _TRANS_HTOTAL_B  0xe1000
-#define _TRANS_HBLANK_B  0xe1004
-#define _TRANS_HSYNC_B   0xe1008
-#define _TRANS_VTOTAL_B  0xe100c
-#define _TRANS_VBLANK_B  0xe1010
-#define _TRANS_VSYNC_B   0xe1014
-#define _TRANS_VSYNCSHIFT_B 0xe1028
-
-#define TRANS_HTOTAL(pipe) _PIPE(pipe, _TRANS_HTOTAL_A, _TRANS_HTOTAL_B)
-#define TRANS_HBLANK(pipe) _PIPE(pipe, _TRANS_HBLANK_A, _TRANS_HBLANK_B)
-#define TRANS_HSYNC(pipe) _PIPE(pipe, _TRANS_HSYNC_A, _TRANS_HSYNC_B)
-#define TRANS_VTOTAL(pipe) _PIPE(pipe, _TRANS_VTOTAL_A, _TRANS_VTOTAL_B)
-#define TRANS_VBLANK(pipe) _PIPE(pipe, _TRANS_VBLANK_A, _TRANS_VBLANK_B)
-#define TRANS_VSYNC(pipe) _PIPE(pipe, _TRANS_VSYNC_A, _TRANS_VSYNC_B)
-#define TRANS_VSYNCSHIFT(pipe) _PIPE(pipe, _TRANS_VSYNCSHIFT_A, \
-_TRANS_VSYNCSHIFT_B)
+#define _PCH_TRANS_HTOTAL_B  0xe1000
+#define _PCH_TRANS_HBLANK_B  0xe1004
+#define _PCH_TRANS_HSYNC_B   0xe1008
+#define _PCH_TRANS_VTOTAL_B  0xe100c
+#define _PCH_TRANS_VBLANK_B  0xe1010
+#define _PCH_TRANS_VSYNC_B   0xe1014
+#define _PCH_TRANS_VSYNCSHIFT_B 0xe1028
+
+#define PCH_TRANS_HTOTAL(pipe) _PIPE(pipe, _PCH_TRANS_HTOTAL_A, 
_PCH_TRANS_HTOTAL_B)
+#define PCH_TRANS_HBLANK(pipe) _PIPE(pipe, _PCH_TRANS_HBLANK_A, 
_PCH_TRANS_HBLANK_B)
+#define PCH_TRANS_HSYNC(pipe) _PIPE(pipe, _PCH_TRANS_HSYNC_A, 
_PCH_TRANS_HSYNC_B)
+#define PCH_TRANS_VTOTAL(pipe) _PIPE(pipe, _PCH_TRANS_VTOTAL_A, 
_PCH_TRANS_VTOTAL_B)
+#define PCH_TRANS_VBLANK(pipe) _PIPE(pipe, _PCH_TRANS_VBLANK_A, 
_PCH_TRANS_VBLANK_B)
+#define PCH_TRANS_VSYNC(pipe) _PIPE(pipe, _PCH_TRANS_VSYNC_A, 
_PCH_TRANS_VSYNC_B)
+#define PCH_TRANS_VSYNCSHIFT(pipe) _PIPE(pipe, _PCH_TRANS_VSYNCSHIFT_A, \
+_PCH_TRANS_VSYNCSHIFT_B)
 
 #define _TRANSB_DATA_M1  0xe1030
 #define _TRANSB_DATA_N1  0xe1034
diff --git a/drivers/gpu/drm/i915/i915_ums.c b/drivers/gpu/drm/i915/i915_ums.c
index 75960dd..4168d2b 100644
--- a/drivers/gpu/drm/i915/i915_ums.c

[Intel-gfx] [PATCH 1/6] drm/i915: s/TRANSCONF/PCH_TRANSCONF/

2013-05-03 Thread Daniel Vetter
Every time I read hsw code I get completely confused about this. So
call it what it is more explicitly.

Also, add an LPT_TRANSCONF for the pch transcoder A and use it in
lpt-only code, to really unconfuse me.

v2: s/plane/pipe/ in the TRANSCONF #define (Paulo).

Reviewed-by: Paulo Zanoni 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_reg.h  |  7 ---
 drivers/gpu/drm/i915/i915_ums.c  |  8 
 drivers/gpu/drm/i915/intel_display.c | 30 +++---
 3 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d84d694..d6fc8e7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4056,9 +4056,10 @@
 #define TRANSDPLINK_M2(pipe) _PIPE(pipe, _TRANSA_DP_LINK_M2, 
_TRANSB_DP_LINK_M2)
 #define TRANSDPLINK_N2(pipe) _PIPE(pipe, _TRANSA_DP_LINK_N2, 
_TRANSB_DP_LINK_N2)
 
-#define _TRANSACONF  0xf0008
-#define _TRANSBCONF  0xf1008
-#define TRANSCONF(plane) _PIPE(plane, _TRANSACONF, _TRANSBCONF)
+#define _PCH_TRANSACONF  0xf0008
+#define _PCH_TRANSBCONF  0xf1008
+#define PCH_TRANSCONF(pipe) _PIPE(pipe, _PCH_TRANSACONF, _PCH_TRANSBCONF)
+#define LPT_TRANSCONF  _PCH_TRANSACONF /* lpt has only one transcoder 
*/
 #define  TRANS_DISABLE  (0<<31)
 #define  TRANS_ENABLE   (1<<31)
 #define  TRANS_STATE_MASK   (1<<30)
diff --git a/drivers/gpu/drm/i915/i915_ums.c b/drivers/gpu/drm/i915/i915_ums.c
index 985a097..75960dd 100644
--- a/drivers/gpu/drm/i915/i915_ums.c
+++ b/drivers/gpu/drm/i915/i915_ums.c
@@ -148,7 +148,7 @@ void i915_save_display_reg(struct drm_device *dev)
dev_priv->regfile.savePFA_WIN_SZ = I915_READ(_PFA_WIN_SZ);
dev_priv->regfile.savePFA_WIN_POS = I915_READ(_PFA_WIN_POS);
 
-   dev_priv->regfile.saveTRANSACONF = I915_READ(_TRANSACONF);
+   dev_priv->regfile.saveTRANSACONF = I915_READ(_PCH_TRANSACONF);
dev_priv->regfile.saveTRANS_HTOTAL_A = 
I915_READ(_TRANS_HTOTAL_A);
dev_priv->regfile.saveTRANS_HBLANK_A = 
I915_READ(_TRANS_HBLANK_A);
dev_priv->regfile.saveTRANS_HSYNC_A = I915_READ(_TRANS_HSYNC_A);
@@ -205,7 +205,7 @@ void i915_save_display_reg(struct drm_device *dev)
dev_priv->regfile.savePFB_WIN_SZ = I915_READ(_PFB_WIN_SZ);
dev_priv->regfile.savePFB_WIN_POS = I915_READ(_PFB_WIN_POS);
 
-   dev_priv->regfile.saveTRANSBCONF = I915_READ(_TRANSBCONF);
+   dev_priv->regfile.saveTRANSBCONF = I915_READ(_PCH_TRANSBCONF);
dev_priv->regfile.saveTRANS_HTOTAL_B = 
I915_READ(_TRANS_HTOTAL_B);
dev_priv->regfile.saveTRANS_HBLANK_B = 
I915_READ(_TRANS_HBLANK_B);
dev_priv->regfile.saveTRANS_HSYNC_B = I915_READ(_TRANS_HSYNC_B);
@@ -379,7 +379,7 @@ void i915_restore_display_reg(struct drm_device *dev)
I915_WRITE(_PFA_WIN_SZ, dev_priv->regfile.savePFA_WIN_SZ);
I915_WRITE(_PFA_WIN_POS, dev_priv->regfile.savePFA_WIN_POS);
 
-   I915_WRITE(_TRANSACONF, dev_priv->regfile.saveTRANSACONF);
+   I915_WRITE(_PCH_TRANSACONF, dev_priv->regfile.saveTRANSACONF);
I915_WRITE(_TRANS_HTOTAL_A, 
dev_priv->regfile.saveTRANS_HTOTAL_A);
I915_WRITE(_TRANS_HBLANK_A, 
dev_priv->regfile.saveTRANS_HBLANK_A);
I915_WRITE(_TRANS_HSYNC_A, dev_priv->regfile.saveTRANS_HSYNC_A);
@@ -448,7 +448,7 @@ void i915_restore_display_reg(struct drm_device *dev)
I915_WRITE(_PFB_WIN_SZ, dev_priv->regfile.savePFB_WIN_SZ);
I915_WRITE(_PFB_WIN_POS, dev_priv->regfile.savePFB_WIN_POS);
 
-   I915_WRITE(_TRANSBCONF, dev_priv->regfile.saveTRANSBCONF);
+   I915_WRITE(_PCH_TRANSBCONF, dev_priv->regfile.saveTRANSBCONF);
I915_WRITE(_TRANS_HTOTAL_B, 
dev_priv->regfile.saveTRANS_HTOTAL_B);
I915_WRITE(_TRANS_HBLANK_B, 
dev_priv->regfile.saveTRANS_HBLANK_B);
I915_WRITE(_TRANS_HSYNC_B, dev_priv->regfile.saveTRANS_HSYNC_B);
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 2939524..9ee2f9e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1206,14 +1206,14 @@ static void assert_pch_refclk_enabled(struct 
drm_i915_private *dev_priv)
WARN(!enabled, "PCH refclk assertion failure, should be active but is 
disabled\n");
 }
 
-static void assert_transcoder_disabled(struct drm_i915_private *dev_priv,
-  enum pipe pipe)
+static void assert_pch_transcoder_disabled(struct drm_i915_private *dev_priv,
+  enum pipe pipe)
 {
int reg;
u32 val;
bool enabled;
 
-   reg = TRANSCONF(pipe);
+   reg = PCH_TRANSCONF(pipe);
val = I915_READ(reg);
enabled 

Re: [Intel-gfx] [PATCH] [TRIVIAL] Fix declaration of intel_gmbus_{is_forced_bit/is_port_falid} in i915 driver.

2013-05-03 Thread Daniel Vetter
On Fri, May 03, 2013 at 11:17:42AM +0200, dl...@gmx.de wrote:
> From: Jan-Simon Möller 
> 
> Description:
> intel_gmbus_is_forced_bit is no extern as its body is right below.
> Likewise for intel_gmbus_is_port_valid.
> 
> This fixes a compilation issue with clang. An initial version of this patch
> was developed by PaX Team .
> This is respin of this patch.
> 
> Signed-off-by: Jan-Simon Möller 
> CC: pagee...@freemail.hu
> CC: daniel.vet...@ffwll.ch
> CC: airl...@linux.ie
> CC: intel-gfx@lists.freedesktop.org
> CC: dri-de...@lists.freedesktop.org
> CC: linux-ker...@vger.kernel.org
Picked up for -fixes, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/5] drm/i915: s/TRANSCONF/PCH_TRANSCONF/

2013-05-03 Thread Daniel Vetter
On Thu, May 02, 2013 at 03:12:07PM -0300, Paulo Zanoni wrote:
> Hi
> 
> 2013/5/1 Daniel Vetter :
> > Every time I read hsw code I get completely confused about this. So
> > call it what it is more explicitly.
> >
> > Also, add an LPT_TRANSCONF for the pch transcoder A and use it in
> > lpt-only code, to really unconfuse me.
> >
> > Signed-off-by: Daniel Vetter 
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h  |  7 ---
> >  drivers/gpu/drm/i915/i915_ums.c  |  8 
> >  drivers/gpu/drm/i915/intel_display.c | 30 +++---
> >  3 files changed, 23 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 95ae5cf..d06a571 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4058,9 +4058,10 @@
> >  #define TRANSDPLINK_M2(pipe) _PIPE(pipe, _TRANSA_DP_LINK_M2, 
> > _TRANSB_DP_LINK_M2)
> >  #define TRANSDPLINK_N2(pipe) _PIPE(pipe, _TRANSA_DP_LINK_N2, 
> > _TRANSB_DP_LINK_N2)
> >
> > -#define _TRANSACONF  0xf0008
> > -#define _TRANSBCONF  0xf1008
> > -#define TRANSCONF(plane) _PIPE(plane, _TRANSACONF, _TRANSBCONF)
> > +#define _PCH_TRANSACONF  0xf0008
> > +#define _PCH_TRANSBCONF  0xf1008
> > +#define PCH_TRANSCONF(plane) _PIPE(plane, _PCH_TRANSACONF, _PCH_TRANSBCONF)
> > +#define LPT_TRANSCONF  _PCH_TRANSACONF /* lpt has only one 
> > transcoder */
> 
> Since we're already touching the macros, I need to point that we call
> the argument "plane" but use the _PIPE macro, when it's actually a
> transcoder :)

Yeah, plane is wrong ;-) But I've opted for PIPE instead, since the
disdinction between transcoder and pipe is only relevant on hsw cpu stuff,
and I very much don't expect the pch to grow an eDP transcoder.

Imo that helps quite a bit since the crtc->pipe mapping is fixed, but the
transcoder stuff depends upon the configuration. Hence why I prefer pipe
if possible.
-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 4/5] drm/i915: Apply OCD to data/link m/n register #defines

2013-05-03 Thread Daniel Vetter
On Thu, May 02, 2013 at 05:36:33PM -0300, Paulo Zanoni wrote:
> Hi
> 
> 2013/5/1 Daniel Vetter :
> > - PCH_ prefix for pch registers on ibx/cpt/ppt.
> > - Drop the DP_ from the link defines, redundant.
> > - Drop the GMCH from the data defines and instead give the special g4x
> >   registers a consistent _G4X postfix.
> >
> > Signed-off-by: Daniel Vetter 
> 
> Yay, the new naming is way better. Thanks!
> 
> So since this is about OCD, I'll give my optional OCD bikesheds:
> - Now the register addresses are not aligned on i915_reg.h anymore. I
> suggest we align them again. We should also consider replacing all
> those white spaces with tabs.

Done.

> - We should have renamed the suspend/resume regfile registers too.
> They're getting confusing now.

Imo i915_ums.c is a dungeon and I don't want to change anything in there
if at all possible. Hence why I've kept the old names.

> - We go over 80 columns in some new cases now. Do we have a policy on
> when it's accepted on our driver?

I don't mind too much if something in the dungeons (i915_ums.c and
i915_dma.c overflows). Same for reg defines, since usually you don't
really read them that often. Real code in source files shouldn't overflow
for easier reading (with some exceptions for strings).

> - Reviewed-by: Paulo Zanoni  but I'd love to
> see my bikesheds on v2 :)

On it's way ;-)
-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] [alsa-devel] [PATCH] drm/i915: Add private api for power well usage -- alignment between graphic team and audio team

2013-05-03 Thread Wang, Xingchao
Hi David,

Thank you very much for your draft patch.
I have some more work on a new patchset, some ideas are from your patch.

Here's a brief introduction of attached patchset:

1. a new bus type in /sound/had_bus.c, used to bind the single module and codec 
device
It looks like ac97_bus.c

2. add a new device node in "struct hda_codec", it's used to register for new 
bus type.

3. a new single module hdmi_i915, which compiled in only when DRM_I915 and 
CODEC_HDMI enabled.
It stores the private API for gfx part.
There's no support to probe haswell hdmi codec only yet. Power well will be 
used only for haswell display audio.

4. power well API implementation in gfx side.

Please feel free to add your idea and I will help test your patch too.

Thanks
--xingchao

> -Original Message-
> From: David Henningsson [mailto:david.hennings...@canonical.com]
> Sent: Thursday, May 02, 2013 10:49 PM
> To: Liam Girdwood
> Cc: Barnes, Jesse; alsa-de...@alsa-project.org; Zanoni, Paulo R; Takashi Iwai;
> Daniel Vetter; intel-gfx@lists.freedesktop.org; Wysocki, Rafael J; Wang
> Xingchao; Wang, Xingchao; Li, Jocelyn; Hindman, Gavin; Daniel Vetter; Lin,
> Mengdong; ville.syrj...@linux.intel.com
> Subject: Re: [alsa-devel] [PATCH] drm/i915: Add private api for power well
> usage -- alignment between graphic team and audio team
> 
> On 04/30/2013 04:41 PM, Liam Girdwood wrote:
> > On Tue, 2013-04-30 at 12:29 +0200, David Henningsson wrote:
> >> On 04/29/2013 05:02 PM, Jesse Barnes wrote:
> >>> On Sat, 27 Apr 2013 13:35:29 +0200
> >>> Daniel Vetter  wrote:
> >>>
>  On Sat, Apr 27, 2013 at 09:20:39AM +, Wang, Xingchao wrote:
> > Let me throw a basic proposal on Audio driver side,  please give your
> comments freely.
> >
> > it contains the power well control usage points:
> > #1: audio request power well at boot up.
> > I915 may shut down power well after bootup initialization, as there's no
> monitor connected outside or only eDP on pipe A.
> > #2: audio request power on resume
> > After exit from D3 mode, audio driver quest power on. This may happen
> at normal resume or runtime resume.
> > #3: audio release power well control at suspend Audio driver will
> > let i915 know it doensot need power well anymore as it's going to
> suspend. This may happened at normal suspend or runtime suspend point.
> > #4: audio release power well when module unload Audio release
> > power well at remove callback to let i915 know.
> 
>  I miss the power well grab/dropping at runtime from the audio side.
>  If the audio driver forces the power well to be on the entire time
>  it's loaded, that's not good, since the power well stuff is very much for
> runtime PM.
>  We _must_ be able to switch off the power well whenever possible.
> >>>
> >>> Xingchao, I'm not an audio developer so I'm probably way off.
> >>>
> >>> But what we really need is a very small and targeted set of calls
> >>> into the i915 driver from say the HDMI driver in HDA.  It looks like
> >>> the prepare/cleanup pair in the pcm_ops structure might be the right
> >>> place to put things?  If that's too fine grained, you could do it at
> >>> open/close time I guess, but the danger there is that some app will
> >>> keep the device open even while it's not playing.
> >>>
> >>> If that won't work, maybe calling i915 from hda_power_work in the
> >>> higher level code would be better?
> >>>
> >>> For detecting whether to call i915 at all, you can filter on the PCI
> >>> IDs (just look for an Intel graphics device and if present, try to
> >>> get the i915 symbols for the power functions).
> >>>
> >>> --- a/sound/pci/hda/hda_codec.c
> >>> +++ b/sound/pci/hda/hda_codec.c
> >>> @@ -3860,6 +3860,8 @@ static unsigned int
> hda_call_codec_suspend(struct hda_code
> >>>   codec->patch_ops.suspend(codec);
> >>>   hda_cleanup_all_streams(codec);
> >>>   state = hda_set_power_state(codec, AC_PWRST_D3);
> >>> +   if (i915_shared_power_well)
> >>> +   i915_put_power_well(codec->i915_data);
> >>>   /* Cancel delayed work if we aren't currently running from it.
> */
> >>>   if (!in_wq)
> >>>   cancel_delayed_work_sync(&codec->power_work);
> >>> @@ -4807,6 +4809,9 @@ static void __snd_hda_power_up(struct
> hda_codec *codec, bo
> >>>   return;
> >>>   spin_unlock(&codec->power_lock);
> >>>
> >>> +   if (i915_shared_power_well)
> >>> +   i915_get_power_well(codec->i915_data);
> >>
> >> Is it wise that a _get function actually has side effects? Perhaps
> >> _push and _pop or something else would be better semantics.
> >>
> >
> > I think the intention here is to model on the PM runtime subsystem
> > where we can get/put the reference count on a PM resource. I'd expect
> > with this API that i915_get_power_well() will increment the count and
> > prevent the shared PM resource from being powered OFF.
> 
> Ok, I stand

Re: [Intel-gfx] [PATCH 2/3] drm/i915: move PCH pfit controls into pipe_config

2013-05-03 Thread Daniel Vetter
On Fri, May 3, 2013 at 12:02 AM, Paulo Zanoni  wrote:
>>  static void valleyview_crtc_enable(struct drm_crtc *crtc)
>> @@ -5800,6 +5798,9 @@ static void haswell_modeset_global_resources(struct 
>> drm_device *dev)
>> /* XXX: Should check for edp transcoder here, but thanks to 
>> init
>>  * sequence that's not yet available. Just in case desktop 
>> eDP
>>  * on PORT D is possible on haswell, too. */
>> +   /* Even the eDP panel fitter is outside the always-on well. 
>> */
>> +   if (I915_READ(PF_WIN_SZ(crtc->pipe)))
>
> So this patch made it to drm-intel-next-queued and the line above
> causes "unclaimed register" errors. This happens even if we don't
> disable the power well, and we were 100% clear of these error messages
> on that case, so I guess I can call this a "regression". Can you
> please provide a fix?

Well, that hunk switches from checking dev_priv->pch_pf_size (which is
now in crtc->config, so we'd need to check the right
pipe/cpu_transcoder first) to the register value, which is just plain
bogus.

While at it we could fix the XXX in haswell_crtc_disable, which is now
possible with the pch pfit tracking.
-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 2/2]i-g-t: gem_dummy_reloc_loop.c: add vebox test case

2013-05-03 Thread Zhong Li
Signed-off-by: Zhong Li 
---
 tests/gem_dummy_reloc_loop.c |   21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/tests/gem_dummy_reloc_loop.c b/tests/gem_dummy_reloc_loop.c
index b67c7d3..b5da577 100644
--- a/tests/gem_dummy_reloc_loop.c
+++ b/tests/gem_dummy_reloc_loop.c
@@ -42,6 +42,8 @@
 #include "intel_gpu_tools.h"
 #include "i830_reg.h"
 
+#define LOCACL_I915_EXEC_VEBOX (4<<0)
+
 static drm_intel_bufmgr *bufmgr;
 struct intel_batchbuffer *batch;
 static drm_intel_bo *target_buffer;
@@ -88,14 +90,14 @@ dummy_reloc_loop(int ring)
 }
 
 static void
-dummy_reloc_loop_random_ring(void)
+dummy_reloc_loop_random_ring(int num_rings)
 {
int i;
 
srandom(0xdeadbeef);
 
for (i = 0; i < 0x10; i++) {
-   int ring = random() % 3 + 1;
+   int ring = random() % num_rings + 1;
 
if (ring == I915_EXEC_RENDER) {
BEGIN_BATCH(4);
@@ -126,11 +128,13 @@ int main(int argc, char **argv)
 {
int fd;
int devid;
+   int num_rings = 1; /*render ring is alwyas available*/
 
drmtest_subtest_init(argc, argv);
 
fd = drm_open_any();
devid = intel_get_drm_devid(fd);
+   num_rings = gem_get_num_rings(fd);
if (!HAS_BLT_RING(devid)) {
fprintf(stderr, "not (yet) implemented for pre-snb\n");
return 77;
@@ -179,11 +183,20 @@ int main(int argc, char **argv)
}
}
 
+   if (drmtest_run_subtest("vebox")) {
+   if (gem_has_vebox(fd)) {
+   sleep(2);
+   printf("running dummy loop on vebox\n");
+   dummy_reloc_loop(LOCACL_I915_EXEC_VEBOX);
+   printf("dummy loop run on vebox completed\n");
+   }
+   }
+
if (drmtest_run_subtest("mixed")) {
-   if (HAS_BLT_RING(devid) && HAS_BSD_RING(devid)) {
+   if (num_rings > 1) {
sleep(2);
printf("running dummy loop on random rings\n");
-   dummy_reloc_loop_random_ring();
+   dummy_reloc_loop_random_ring(num_rings);
printf("dummy loop run on random rings completed\n");
}
}
-- 
1.7.9.5

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


[Intel-gfx] [PATCH 1/2]i-g-t: check kernel enable rings or not

2013-05-03 Thread Zhong Li
Signed-off-by: Zhong Li 

1. add functions check kernel enable a ring or not.
2. add function gem_get_num_rings() to check how many rings kernel has enable.
3. gem_ring_sync_loop.c will call gem_get_num_rings() directly instead of 
original static fucntion get_number_rings().
---
 lib/drmtest.c  |   61 ++--
 lib/drmtest.h  |6 -
 tests/gem_ring_sync_loop.c |   38 +--
 3 files changed, 59 insertions(+), 46 deletions(-)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index 2ddaff0..a6a988b 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -270,19 +270,64 @@ void gem_set_tiling(int fd, uint32_t handle, int tiling, 
int stride)
assert(st.tiling_mode == tiling);
 }
 
+bool gem_has_enable_ring(int fd,int param)
+{
+   drm_i915_getparam_t gp;
+   int ret, tmp;
+   memset(&gp, 0, sizeof(gp));
+   
+   gp.value = &tmp;
+   gp.param = param;
+   
+   ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
+   
+   if ((ret == 0) && (*gp.value > 0))
+   return true;
+   else
+   return false;
+}
+
+bool gem_has_bsd(int fd)
+{
+
+   return gem_has_enable_ring(fd,I915_PARAM_HAS_BSD);
+}
+
+bool gem_has_blt(int fd)
+{
+
+   return gem_has_enable_ring(fd,I915_PARAM_HAS_BLT);
+}
+
 #define LOCAL_I915_PARAM_HAS_VEBOX 22
-int gem_has_vebox(int fd)
+bool gem_has_vebox(int fd)
 {
-   struct drm_i915_getparam gp;
-   int val;
 
-   gp.param = LOCAL_I915_PARAM_HAS_VEBOX;
-   gp.value = &val;
+   return gem_has_enable_ring(fd,LOCAL_I915_PARAM_HAS_VEBOX);
+}
+
+int gem_get_num_rings(int fd)
+{
+   int num_rings = 1;  /* render ring is always available */
+
+   if (gem_has_bsd(fd))
+   num_rings++;
+   else
+   goto skip;
+
+   if (gem_has_blt(fd))
+   num_rings++;
+   else
+   goto skip;
+
+   if (gem_has_vebox(fd))
+   num_rings++;
+   else
+   goto skip;
 
-   if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp)))
-   return 0;
 
-   return val != 0;
+skip:
+   return num_rings;
 }
 
 struct local_drm_i915_gem_cacheing {
diff --git a/lib/drmtest.h b/lib/drmtest.h
index f15c074..3549c47 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -47,8 +47,12 @@ int drm_open_any_master(void);
 void gem_quiescent_gpu(int fd);
 
 /* ioctl wrappers and similar stuff for bare metal testing */
-int gem_has_vebox(int fd);
 void gem_set_tiling(int fd, uint32_t handle, int tiling, int stride);
+bool gem_has_enable_ring(int fd,int param);
+bool gem_has_bsd(int fd);
+bool gem_has_blt(int fd);
+bool gem_has_vebox(int fd);
+int gem_get_num_rings(int fd);
 int gem_has_cacheing(int fd);
 void gem_set_cacheing(int fd, uint32_t handle, int cacheing);
 int gem_get_cacheing(int fd, uint32_t handle);
diff --git a/tests/gem_ring_sync_loop.c b/tests/gem_ring_sync_loop.c
index 501e97c..af40590 100644
--- a/tests/gem_ring_sync_loop.c
+++ b/tests/gem_ring_sync_loop.c
@@ -55,47 +55,11 @@ static drm_intel_bo *target_buffer;
 #define MI_COND_BATCH_BUFFER_END   (0x36<<23 | 1)
 #define MI_DO_COMPARE  (1<<21)
 
-static int
-get_num_rings(int fd)
-{
-   int num_rings = 1;  /* render ring is always available */
-   drm_i915_getparam_t gp;
-   int ret, tmp;
-
-   memset(&gp, 0, sizeof(gp));
-   gp.value = &tmp;
-
-   gp.param = I915_PARAM_HAS_BSD;
-   ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
-
-   if ((ret == 0) && (*gp.value > 0))
-   num_rings++;
-   else
-   goto skip;
-
-   gp.param = I915_PARAM_HAS_BLT;
-   ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
-
-   if ((ret == 0) && (*gp.value > 0))
-   num_rings++;
-   else
-   goto skip;
-
-   if (gem_has_vebox(fd))
-   num_rings++;
-   else
-   goto skip;
-
-
-skip:
-   return num_rings;
-}
-
 static void
 store_dword_loop(int fd)
 {
int i;
-   int num_rings = get_num_rings(fd);
+   int num_rings = gem_get_num_rings(fd);
 
srandom(0xdeadbeef);
 
-- 
1.7.9.5

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


Re: [Intel-gfx] [PATCH 2/2]i-g-t: gem_dummy_reloc_loop.c: add vebox test case Signed-off-by: Zhong Li

2013-05-03 Thread Li, Zhong
I made a mistake for the patch subject. I will send a new series of patch. 
Please ignore this patch.

-Original Message-
From: Li, Zhong 
Sent: Friday, May 03, 2013 2:26 PM
To: intel-gfx@lists.freedesktop.org
Cc: Li, Zhong; Xiang, Haihao; Li, Jocelyn; Vetter, Daniel; b...@bwidawsk.net
Subject: [Intel-gfx][PATCH 2/2]i-g-t: gem_dummy_reloc_loop.c: add vebox test 
case Signed-off-by: Zhong Li 

---
 tests/gem_dummy_reloc_loop.c |   21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/tests/gem_dummy_reloc_loop.c b/tests/gem_dummy_reloc_loop.c index 
b67c7d3..c56d559 100644
--- a/tests/gem_dummy_reloc_loop.c
+++ b/tests/gem_dummy_reloc_loop.c
@@ -42,6 +42,8 @@
 #include "intel_gpu_tools.h"
 #include "i830_reg.h"
 
+#define LOCACL_I915_EXEC_VEBOX (4<<0)
+
 static drm_intel_bufmgr *bufmgr;
 struct intel_batchbuffer *batch;
 static drm_intel_bo *target_buffer;
@@ -88,14 +90,14 @@ dummy_reloc_loop(int ring)  }
 
 static void
-dummy_reloc_loop_random_ring(void)
+dummy_reloc_loop_random_ring(int fd, int num_rings)
 {
int i;
 
srandom(0xdeadbeef);
 
for (i = 0; i < 0x10; i++) {
-   int ring = random() % 3 + 1;
+   int ring = random() % num_rings + 1;
 
if (ring == I915_EXEC_RENDER) {
BEGIN_BATCH(4);
@@ -126,11 +128,13 @@ int main(int argc, char **argv)  {
int fd;
int devid;
+   int num_rings = 1; /*render ring is alwyas available*/
 
drmtest_subtest_init(argc, argv);
 
fd = drm_open_any();
devid = intel_get_drm_devid(fd);
+   num_rings = gem_get_num_rings(fd);
if (!HAS_BLT_RING(devid)) {
fprintf(stderr, "not (yet) implemented for pre-snb\n");
return 77;
@@ -179,11 +183,20 @@ int main(int argc, char **argv)
}
}
 
+   if (drmtest_run_subtest("vebox")) {
+   if (gem_has_vebox(fd)) {
+   sleep(2);
+   printf("running dummy loop on vebox\n");
+   dummy_reloc_loop(LOCACL_I915_EXEC_VEBOX);
+   printf("dummy loop run on vebox completed\n");
+   }
+   }
+
if (drmtest_run_subtest("mixed")) {
-   if (HAS_BLT_RING(devid) && HAS_BSD_RING(devid)) {
+   if (num_rings > 1) {
sleep(2);
printf("running dummy loop on random rings\n");
-   dummy_reloc_loop_random_ring();
+   dummy_reloc_loop_random_ring(fd, num_rings);
printf("dummy loop run on random rings completed\n");
}
}
--
1.7.9.5

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


Re: [Intel-gfx] [PATCH 1/2]i-g-t: check kernel enable rings or not Signed-off-by: Zhong Li 1. add functions check kernel enable a ring or not. 2. add function gem_get_num_rings()

2013-05-03 Thread Li, Zhong
I made a mistake for the patch subject and drmtest.h. I will send a new series 
of patch. Please ignore this patch.

-Original Message-
From: Li, Zhong 
Sent: Friday, May 03, 2013 2:26 PM
To: intel-gfx@lists.freedesktop.org
Cc: Li, Zhong; Xiang, Haihao; Li, Jocelyn; Vetter, Daniel; b...@bwidawsk.net
Subject: [Intel-gfx][PATCH 1/2]i-g-t: check kernel enable rings or not 
Signed-off-by: Zhong Li  1. add functions check kernel 
enable a ring or not. 2. add function gem_get_num_rings() to check how many 
rings kernel has enable. 3. gem_ring_sy...

---
 lib/drmtest.c  |   61 ++--
 lib/drmtest.h  |6 -
 tests/gem_ring_sync_loop.c |   38 +--
 3 files changed, 59 insertions(+), 46 deletions(-)

diff --git a/lib/drmtest.c b/lib/drmtest.c index 2ddaff0..ea308b7 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -270,19 +270,64 @@ void gem_set_tiling(int fd, uint32_t handle, int tiling, 
int stride)
assert(st.tiling_mode == tiling);
 }
 
+bool gem_has_enable_ring(int fd,int param) {
+   drm_i915_getparam_t gp;
+   int ret, tmp;
+   memset(&gp, 0, sizeof(gp));
+   
+   gp.value = &tmp;
+   gp.param = param;
+   
+   ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
+   
+   if ((ret == 0) && (*gp.value > 0))
+   return true;
+   else
+   return false;
+}
+
+bool gem_has_bsd(int fd)
+{
+
+   return gem_has_enable_ring(fd,I915_PARAM_HAS_BSD);
+}
+
+bool gem_has_blt(int fd)
+{
+
+   return gem_has_enable_ring(fd,I915_PARAM_HAS_BLT);
+}
+
 #define LOCAL_I915_PARAM_HAS_VEBOX 22
-int gem_has_vebox(int fd)
+bool gem_has_vebox(int fd)
 {
-   struct drm_i915_getparam gp;
-   int val;
 
-   gp.param = LOCAL_I915_PARAM_HAS_VEBOX;
-   gp.value = &val;
+   return gem_has_enable_ring(fd,LOCAL_I915_PARAM_HAS_VEBOX);
+}
+
+int gem_get_num_rings(int fd)
+{
+   int num_rings = 1;  /* render ring is always available */
+
+   if (gem_has_bsd(fd))
+   num_rings++;
+   else
+   goto skip;
+
+   if (gem_has_blt(fd)
+   num_rings++;
+   else
+   goto skip;
+
+   if (gem_has_vebox(fd))
+   num_rings++;
+   else
+   goto skip;
 
-   if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp)))
-   return 0;
 
-   return val != 0;
+skip:
+   return num_rings;
 }
 
 struct local_drm_i915_gem_cacheing {
diff --git a/lib/drmtest.h b/lib/drmtest.h index f15c074..18aeb85 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -47,8 +47,12 @@ int drm_open_any_master(void);  void gem_quiescent_gpu(int 
fd);
 
 /* ioctl wrappers and similar stuff for bare metal testing */ -int 
gem_has_vebox(int fd);  void gem_set_tiling(int fd, uint32_t handle, int 
tiling, int stride);
+bool gem_has_enable_ring(int fd,int param); bool gem_has_bsd(int fd); 
+bool gem_has_bls(int fd); bool gem_has_vebox(int fd); int 
+gem_get_num_rings(int fd);
 int gem_has_cacheing(int fd);
 void gem_set_cacheing(int fd, uint32_t handle, int cacheing);  int 
gem_get_cacheing(int fd, uint32_t handle); diff --git 
a/tests/gem_ring_sync_loop.c b/tests/gem_ring_sync_loop.c index 
501e97c..af40590 100644
--- a/tests/gem_ring_sync_loop.c
+++ b/tests/gem_ring_sync_loop.c
@@ -55,47 +55,11 @@ static drm_intel_bo *target_buffer;
 #define MI_COND_BATCH_BUFFER_END   (0x36<<23 | 1)
 #define MI_DO_COMPARE  (1<<21)
 
-static int
-get_num_rings(int fd)
-{
-   int num_rings = 1;  /* render ring is always available */
-   drm_i915_getparam_t gp;
-   int ret, tmp;
-
-   memset(&gp, 0, sizeof(gp));
-   gp.value = &tmp;
-
-   gp.param = I915_PARAM_HAS_BSD;
-   ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
-
-   if ((ret == 0) && (*gp.value > 0))
-   num_rings++;
-   else
-   goto skip;
-
-   gp.param = I915_PARAM_HAS_BLT;
-   ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
-
-   if ((ret == 0) && (*gp.value > 0))
-   num_rings++;
-   else
-   goto skip;
-
-   if (gem_has_vebox(fd))
-   num_rings++;
-   else
-   goto skip;
-
-
-skip:
-   return num_rings;
-}
-
 static void
 store_dword_loop(int fd)
 {
int i;
-   int num_rings = get_num_rings(fd);
+   int num_rings = gem_get_num_rings(fd);
 
srandom(0xdeadbeef);
 
--
1.7.9.5

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