[PATCH 0/4] drm-intel-collector - review request

[PATCH 0/4] drm-intel-collector - review request

[Rodrigo Vivi]

This is another drm-intel-collector push for review:
http://cgit.freedesktop.org/~vivijim/drm-intel/log/?h=drm-intel-collector

Here goes the list in order for better reviewers assignment:

Patch     drm/i915: check that the i965g/gm 4G limit is really obeyed - Reviewer: Damien Lespiau <damien.lespiau@intel.com>
Patch     drm/i915: Asynchronously perform the set-base for a simple modeset - Reviewer: I would prefer Ville review this since he had concerns about sorround code. switch to fbcon is much faster with this patch so I let it here and will try to review by myself if I heard nothing else until next update.
Patch     drm/i915: implement another plane WM workaround for HSW - Reviewer: If no body else volunteer I'll do this review until next update.
Patch     drm/i915: Use the real cpu max frequency for ring scaling - Reviewed by Chris.

Overall Process:

drm-intel-collector - review request
 1. Daniel pushs drm-intel-testing (every 2 weeks)
 2. I rebase drm-intel-collector onto drm-intel-testing
 3. Add Reviewer: tag with voluntered reviewers. If you don't believe you should be assigned on a particular patch please don't get mad just tell you wont review or volunteer someone else.
 4. I resubmit remaining patches for review without picking any new (drm-intel-collector - review request)

drm-intel-collector - updated
 5. One week later I collect all simple (1-2) patches that wasn't yet reviewed and not queued by Daniel from one testing update until another.
 6. Request automated QA's PRTS automated i-g-t tests comparing drm-intel-testing x drm-intel-collector
 7. If tests are ok I send the update notification or the patches as a series to intel-gfx mailing list for better tracking and to be reviewed. (drm-intel-collector - updated)
 8. Let me know volunteers for review new patches and also let me know if I've picked any patch that I shouldn't.

There are some reasons that some patches can be left behind:
1. Your patch didn't applied cleanly and I couldn't easily solve the conflicts.
2. Kernel didn't compiled with your patch.
3. I simply missed it. If you believe this is the case please warn me.

Please help me to get these patches reviewed and queued by Daniel.

Also, please let me know if you have further ideas how to improve this process.

Thanks in advance,
Rodrigo.


Ben Widawsky (1):
  drm/i915: Use the real cpu max frequency for ring scaling

Chris Wilson (1):
  drm/i915: Asynchronously perform the set-base for a simple modeset

Daniel Vetter (1):
  drm/i915: check that the i965g/gm 4G limit is really obeyed

Paulo Zanoni (1):
  drm/i915: implement another plane WM workaround for HSW

 drivers/gpu/drm/i915/i915_gem.c      |  3 +++
 drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++----
 drivers/gpu/drm/i915/intel_pm.c      | 17 +++++++++++------
 3 files changed, 30 insertions(+), 10 deletions(-)

-- 
1.8.1.4

[PATCH 1/4] drm/i915: check that the i965g/gm 4G limit is really obeyed

[Rodrigo Vivi]

From: Daniel Vetter <daniel.vetter@ffwll.ch>

In truly crazy circumstances shmem might give us the wrong type of
page. So be a bit paranoid and double check this.

Reviewer: Damien Lespiau <damien.lespiau@intel.com>
Cc: Rob Clark <robdclark@gmail.com>
References: http://lkml.org/lkml/2011/7/11/238
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 19ecfa8..692ebf7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1903,6 +1903,9 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
 			sg->length += PAGE_SIZE;
 		}
 		last_pfn = page_to_pfn(page);
+
+		/* Check that the i965g/gm workaround works. */
+		WARN_ON((gfp & __GFP_DMA32) && (last_pfn >= 0x00100000UL));
 	}
 #ifdef CONFIG_SWIOTLB
 	if (!swiotlb_nr_tbl())
-- 
1.8.1.4

Re: [PATCH 1/4] drm/i915: check that the i965g/gm 4G limit is really obeyed

[Damien Lespiau]

On Mon, Oct 07, 2013 at 05:15:45PM -0300, Rodrigo Vivi wrote:
> From: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> In truly crazy circumstances shmem might give us the wrong type of
> page. So be a bit paranoid and double check this.
> 
> Reviewer: Damien Lespiau <damien.lespiau@intel.com>
> Cc: Rob Clark <robdclark@gmail.com>
> References: http://lkml.org/lkml/2011/7/11/238
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 19ecfa8..692ebf7 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1903,6 +1903,9 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
>  			sg->length += PAGE_SIZE;
>  		}
>  		last_pfn = page_to_pfn(page);
> +
> +		/* Check that the i965g/gm workaround works. */
> +		WARN_ON((gfp & __GFP_DMA32) && (last_pfn >= 0x00100000UL));

I guess we could have something like last_pfn >= (GB(4) >> PAGE_SHIFT)
but in any case:

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

Re: [PATCH 1/4] drm/i915: check that the i965g/gm 4G limit is really obeyed

[Daniel Vetter]

On Tue, Oct 08, 2013 at 12:06:24PM +0100, Damien Lespiau wrote:
> On Mon, Oct 07, 2013 at 05:15:45PM -0300, Rodrigo Vivi wrote:
> > From: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > In truly crazy circumstances shmem might give us the wrong type of
> > page. So be a bit paranoid and double check this.
> > 
> > Reviewer: Damien Lespiau <damien.lespiau@intel.com>
> > Cc: Rob Clark <robdclark@gmail.com>
> > References: http://lkml.org/lkml/2011/7/11/238
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 19ecfa8..692ebf7 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -1903,6 +1903,9 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
> >  			sg->length += PAGE_SIZE;
> >  		}
> >  		last_pfn = page_to_pfn(page);
> > +
> > +		/* Check that the i965g/gm workaround works. */
> > +		WARN_ON((gfp & __GFP_DMA32) && (last_pfn >= 0x00100000UL));
> 
> I guess we could have something like last_pfn >= (GB(4) >> PAGE_SHIFT)
> but in any case:

I was lazy - this is copypasta from gma500 ...
> 
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

Queued for -next, thanks for the review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 2/4] drm/i915: Asynchronously perform the set-base for a simple modeset

[Rodrigo Vivi]

From: Chris Wilson <chris@chris-wilson.co.uk>

A simple modeset, where we only wish to switch over to a new framebuffer
such as the transition from fbcon to X, takes around 30-60ms. This is
due to three factors:

1. We need to make sure the fb->obj is in the display domain, which
incurs a cache flush to ensure no dirt is left on the scanout.

2. We need to flush any pending rendering before performing the mmio
so that the frame is complete before it is shown.

3. We currently wait for the vblank after the mmio to be sure that the
old fb is no longer being shown before releasing it.

(1) can only be eliminated by userspace preparing the fb->obj in advance
to already be in the display domain. This can be done through use of the
create2 ioctl, or by reusing an existing fb->obj.

However, (2) and (3) are already solved by the existing page flip
mechanism, and it is surprisingly trivial to wire them up for use in the
set-base fast path. Though it can be argued that this represents a
subtle ABI break in that the set_config ioctl now returns before the old
framebuffer is unpinned. The danger is that userspace will start to
modify it before it is no longer being shown, however we should be able
to prevent that through proper domain tracking.

By combining all of the above, we can achieve an instaneous set_config:

[     6.601] (II) intel(0): switch to mode 2560x1440@60.0 on pipe 0 using DP2, position (0, 0), rotation normal
[     6.601] (II) intel(0): Setting screen physical size to 677 x 381

v2 (by Vivi): page_flip_flag was added to intel_crtc_page_flip
              in a previous commit. using 0.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/intel_display.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b9cede7..bc47f1e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9569,10 +9569,13 @@ 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);
 	} else if (config->fb_changed) {
-		intel_crtc_wait_for_pending_flips(set->crtc);
-
-		ret = intel_pipe_set_base(set->crtc,
-					  set->x, set->y, set->fb);
+		if (to_intel_framebuffer(set->fb)->obj->ring == NULL ||
+		    save_set.x != set->x || save_set.y != set->y ||
+		    intel_crtc_page_flip(set->crtc, set->fb, NULL, 0)) {
+			intel_crtc_wait_for_pending_flips(set->crtc);
+			ret = intel_pipe_set_base(set->crtc,
+						  set->x, set->y, set->fb);
+		}
 	}
 
 	if (ret) {
-- 
1.8.1.4

Re: [PATCH 2/4] drm/i915: Asynchronously perform the set-base for a simple modeset

[Rodrigo Vivi]

For me this patch makes sense and swith to fbcon is faster, so:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

with this -collector is "empty"... at least until next -testing update.

On Mon, Oct 7, 2013 at 5:15 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
>
> A simple modeset, where we only wish to switch over to a new framebuffer
> such as the transition from fbcon to X, takes around 30-60ms. This is
> due to three factors:
>
> 1. We need to make sure the fb->obj is in the display domain, which
> incurs a cache flush to ensure no dirt is left on the scanout.
>
> 2. We need to flush any pending rendering before performing the mmio
> so that the frame is complete before it is shown.
>
> 3. We currently wait for the vblank after the mmio to be sure that the
> old fb is no longer being shown before releasing it.
>
> (1) can only be eliminated by userspace preparing the fb->obj in advance
> to already be in the display domain. This can be done through use of the
> create2 ioctl, or by reusing an existing fb->obj.
>
> However, (2) and (3) are already solved by the existing page flip
> mechanism, and it is surprisingly trivial to wire them up for use in the
> set-base fast path. Though it can be argued that this represents a
> subtle ABI break in that the set_config ioctl now returns before the old
> framebuffer is unpinned. The danger is that userspace will start to
> modify it before it is no longer being shown, however we should be able
> to prevent that through proper domain tracking.
>
> By combining all of the above, we can achieve an instaneous set_config:
>
> [     6.601] (II) intel(0): switch to mode 2560x1440@60.0 on pipe 0 using DP2, position (0, 0), rotation normal
> [     6.601] (II) intel(0): Setting screen physical size to 677 x 381
>
> v2 (by Vivi): page_flip_flag was added to intel_crtc_page_flip
>               in a previous commit. using 0.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b9cede7..bc47f1e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9569,10 +9569,13 @@ 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);
>         } else if (config->fb_changed) {
> -               intel_crtc_wait_for_pending_flips(set->crtc);
> -
> -               ret = intel_pipe_set_base(set->crtc,
> -                                         set->x, set->y, set->fb);
> +               if (to_intel_framebuffer(set->fb)->obj->ring == NULL ||
> +                   save_set.x != set->x || save_set.y != set->y ||
> +                   intel_crtc_page_flip(set->crtc, set->fb, NULL, 0)) {
> +                       intel_crtc_wait_for_pending_flips(set->crtc);
> +                       ret = intel_pipe_set_base(set->crtc,
> +                                                 set->x, set->y, set->fb);
> +               }
>         }
>
>         if (ret) {
> --
> 1.8.1.4
>



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

[PATCH 3/4] drm/i915: implement another plane WM workaround for HSW

[Rodrigo Vivi]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

In some Haswell machines we're seeing a full system hang while calling
haswell_crtc_enable. Ville bisected the problem to the following
commit:
    commit 90a8864320b2a9f91e5b5d561924a4bb70b90dcc
    Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
    Date:   Fri May 3 17:23:45 2013 -0300
        drm/i915: set FORCE_ARB_IDLE_PLANES workaround

After some BSpec-digging I discovered that we don't implement one of
the workarounds mentioned in the description of bit 31 of PRI_CTL,
SPR_CTL and CUR_CTL. This patch implements the workaround, which makes
the problem go away on my machine. Also notice that the workaround
implementation is almost a revert of the commit mentioned above, but
it still allows LP watermarks to be used.

Thanks to Ville for the help debugging the issue and for doing the
bisect.

Cc: stable@vger.kernel.org
Credits-to: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/intel_display.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bc47f1e..8069bff 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3565,6 +3565,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_encoder *encoder;
 	int pipe = intel_crtc->pipe;
+	uint32_t wm_dbg_val;
 
 	WARN_ON(!crtc->enabled);
 
@@ -3597,6 +3598,11 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	intel_ddi_set_pipe_settings(crtc);
 	intel_ddi_enable_transcoder_func(crtc);
 
+	/* Workaround described in PRI_CTL, CUR_CTL and SPR_CTL bit 31. */
+	wm_dbg_val = I915_READ(WM_DBG);
+	I915_WRITE(WM_DBG, wm_dbg_val | WM_DBG_DISALLOW_MULTIPLE_LP |
+			   WM_DBG_DISALLOW_MAXFIFO | WM_DBG_DISALLOW_SPRITE);
+
 	intel_update_watermarks(crtc);
 	intel_enable_pipe(dev_priv, pipe,
 			  intel_crtc->config.has_pch_encoder, false);
@@ -3623,6 +3629,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	 * happening.
 	 */
 	intel_wait_for_vblank(dev, intel_crtc->pipe);
+
+	/* Second part of the WM_DBG workaround. */
+	I915_WRITE(WM_DBG, wm_dbg_val);
 }
 
 static void ironlake_pfit_disable(struct intel_crtc *crtc)
-- 
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 3/4] drm/i915: implement another plane WM workaround for HSW

[Ville Syrjälä]

On Mon, Oct 07, 2013 at 05:15:47PM -0300, Rodrigo Vivi wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> In some Haswell machines we're seeing a full system hang while calling
> haswell_crtc_enable. Ville bisected the problem to the following
> commit:
>     commit 90a8864320b2a9f91e5b5d561924a4bb70b90dcc
>     Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
>     Date:   Fri May 3 17:23:45 2013 -0300
>         drm/i915: set FORCE_ARB_IDLE_PLANES workaround
> 
> After some BSpec-digging I discovered that we don't implement one of
> the workarounds mentioned in the description of bit 31 of PRI_CTL,
> SPR_CTL and CUR_CTL. This patch implements the workaround, which makes
> the problem go away on my machine. Also notice that the workaround
> implementation is almost a revert of the commit mentioned above, but
> it still allows LP watermarks to be used.
> 
> Thanks to Ville for the help debugging the issue and for doing the
> bisect.

You can drop this patch. It's not needed anymore.

> 
> Cc: stable@vger.kernel.org
> Credits-to: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index bc47f1e..8069bff 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3565,6 +3565,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct intel_encoder *encoder;
>  	int pipe = intel_crtc->pipe;
> +	uint32_t wm_dbg_val;
>  
>  	WARN_ON(!crtc->enabled);
>  
> @@ -3597,6 +3598,11 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>  	intel_ddi_set_pipe_settings(crtc);
>  	intel_ddi_enable_transcoder_func(crtc);
>  
> +	/* Workaround described in PRI_CTL, CUR_CTL and SPR_CTL bit 31. */
> +	wm_dbg_val = I915_READ(WM_DBG);
> +	I915_WRITE(WM_DBG, wm_dbg_val | WM_DBG_DISALLOW_MULTIPLE_LP |
> +			   WM_DBG_DISALLOW_MAXFIFO | WM_DBG_DISALLOW_SPRITE);
> +
>  	intel_update_watermarks(crtc);
>  	intel_enable_pipe(dev_priv, pipe,
>  			  intel_crtc->config.has_pch_encoder, false);
> @@ -3623,6 +3629,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>  	 * happening.
>  	 */
>  	intel_wait_for_vblank(dev, intel_crtc->pipe);
> +
> +	/* Second part of the WM_DBG workaround. */
> +	I915_WRITE(WM_DBG, wm_dbg_val);
>  }
>  
>  static void ironlake_pfit_disable(struct intel_crtc *crtc)
> -- 
> 1.8.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH 4/4] drm/i915: Use the real cpu max frequency for ring scaling

[Rodrigo Vivi]

From: Ben Widawsky <benjamin.widawsky@intel.com>

The policy's max frequency is not equal to the CPU's max frequency. The
ring frequency is derived from the CPU frequency, and not the policy
frequency.

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

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

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

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

    drm/i915: load a ring frequency scaling table v3

CC: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

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

Re: [PATCH 4/4] drm/i915: Use the real cpu max frequency for ring scaling

[Daniel Vetter]

On Mon, Oct 07, 2013 at 05:15:48PM -0300, Rodrigo Vivi wrote:
> From: Ben Widawsky <benjamin.widawsky@intel.com>
> 
> The policy's max frequency is not equal to the CPU's max frequency. The
> ring frequency is derived from the CPU frequency, and not the policy
> frequency.
> 
> One example of how this may differ through sysfs. If the sysfs max
> frequency is modified, that will be used for the max ring frequency
> calculation.
> (/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq). As far as I
> know, no current governor uses anything but max as the default, but in
> theory, they could. Similarly distributions might set policy as part of
> their init process.
> 
> It's ideal to use the real frequency because when we're currently scaled
> up on the GPU. In this case we likely want to race to idle, and using a
> less than max ring frequency is non-optimal for this situation.
> 
> AFAIK, this patch should have no impact on a majority of people.
> 
> This behavior hasn't been changed since it was first introduced:
> commit 23b2f8bb92feb83127679c53633def32d3108e70
> Author: Jesse Barnes <jbarnes@virtuousgeek.org>
> Date:   Tue Jun 28 13:04:16 2011 -0700
> 
>     drm/i915: load a ring frequency scaling table v3
> 
> CC: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 0/4] drm-intel-collector - review request

[Ben Widawsky]

On Mon, Oct 07, 2013 at 05:15:44PM -0300, Rodrigo Vivi wrote:
> 
> This is another drm-intel-collector push for review:
> http://cgit.freedesktop.org/~vivijim/drm-intel/log/?h=drm-intel-collector
> 
> Here goes the list in order for better reviewers assignment:
> 
> Patch     drm/i915: check that the i965g/gm 4G limit is really obeyed - Reviewer: Damien Lespiau <damien.lespiau@intel.com>
> Patch     drm/i915: Asynchronously perform the set-base for a simple modeset - Reviewer: I would prefer Ville review this since he had concerns about sorround code. switch to fbcon is much faster with this patch so I let it here and will try to review by myself if I heard nothing else until next update.
> Patch     drm/i915: implement another plane WM workaround for HSW - Reviewer: If no body else volunteer I'll do this review until next update.
> Patch     drm/i915: Use the real cpu max frequency for ring scaling - Reviewed by Chris.
> 
> Overall Process:
> 
> drm-intel-collector - review request
>  1. Daniel pushs drm-intel-testing (every 2 weeks)
>  2. I rebase drm-intel-collector onto drm-intel-testing
>  3. Add Reviewer: tag with voluntered reviewers. If you don't believe you should be assigned on a particular patch please don't get mad just tell you wont review or volunteer someone else.
>  4. I resubmit remaining patches for review without picking any new (drm-intel-collector - review request)
> 
> drm-intel-collector - updated
>  5. One week later I collect all simple (1-2) patches that wasn't yet reviewed and not queued by Daniel from one testing update until another.
>  6. Request automated QA's PRTS automated i-g-t tests comparing drm-intel-testing x drm-intel-collector
>  7. If tests are ok I send the update notification or the patches as a series to intel-gfx mailing list for better tracking and to be reviewed. (drm-intel-collector - updated)
>  8. Let me know volunteers for review new patches and also let me know if I've picked any patch that I shouldn't.
> 
> There are some reasons that some patches can be left behind:
> 1. Your patch didn't applied cleanly and I couldn't easily solve the conflicts.
> 2. Kernel didn't compiled with your patch.
> 3. I simply missed it. If you believe this is the case please warn me.
> 
> Please help me to get these patches reviewed and queued by Daniel.
> 
> Also, please let me know if you have further ideas how to improve this process.
> 
> Thanks in advance,
> Rodrigo.
> 
> 
> Ben Widawsky (1):
>   drm/i915: Use the real cpu max frequency for ring scaling
> 
> Chris Wilson (1):
>   drm/i915: Asynchronously perform the set-base for a simple modeset
> 
> Daniel Vetter (1):
>   drm/i915: check that the i965g/gm 4G limit is really obeyed
> 
> Paulo Zanoni (1):
>   drm/i915: implement another plane WM workaround for HSW
> 
>  drivers/gpu/drm/i915/i915_gem.c      |  3 +++
>  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++----
>  drivers/gpu/drm/i915/intel_pm.c      | 17 +++++++++++------
>  3 files changed, 30 insertions(+), 10 deletions(-)
> 

Hi Rodrigo. Thanks for doing this. Could I make a small request - can
you please CC all the relevant people when you send these mails?

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 0/4] drm-intel-collector - review request

[Daniel Vetter]

On Mon, Oct 07, 2013 at 09:25:44PM -0700, Ben Widawsky wrote:
> On Mon, Oct 07, 2013 at 05:15:44PM -0300, Rodrigo Vivi wrote:
> > 
> > This is another drm-intel-collector push for review:
> > http://cgit.freedesktop.org/~vivijim/drm-intel/log/?h=drm-intel-collector
> > 
> > Here goes the list in order for better reviewers assignment:
> > 
> > Patch     drm/i915: check that the i965g/gm 4G limit is really obeyed - Reviewer: Damien Lespiau <damien.lespiau@intel.com>
> > Patch     drm/i915: Asynchronously perform the set-base for a simple modeset - Reviewer: I would prefer Ville review this since he had concerns about sorround code. switch to fbcon is much faster with this patch so I let it here and will try to review by myself if I heard nothing else until next update.
> > Patch     drm/i915: implement another plane WM workaround for HSW - Reviewer: If no body else volunteer I'll do this review until next update.
> > Patch     drm/i915: Use the real cpu max frequency for ring scaling - Reviewed by Chris.
> > 
> > Overall Process:
> > 
> > drm-intel-collector - review request
> >  1. Daniel pushs drm-intel-testing (every 2 weeks)
> >  2. I rebase drm-intel-collector onto drm-intel-testing
> >  3. Add Reviewer: tag with voluntered reviewers. If you don't believe you should be assigned on a particular patch please don't get mad just tell you wont review or volunteer someone else.
> >  4. I resubmit remaining patches for review without picking any new (drm-intel-collector - review request)
> > 
> > drm-intel-collector - updated
> >  5. One week later I collect all simple (1-2) patches that wasn't yet reviewed and not queued by Daniel from one testing update until another.
> >  6. Request automated QA's PRTS automated i-g-t tests comparing drm-intel-testing x drm-intel-collector
> >  7. If tests are ok I send the update notification or the patches as a series to intel-gfx mailing list for better tracking and to be reviewed. (drm-intel-collector - updated)
> >  8. Let me know volunteers for review new patches and also let me know if I've picked any patch that I shouldn't.
> > 
> > There are some reasons that some patches can be left behind:
> > 1. Your patch didn't applied cleanly and I couldn't easily solve the conflicts.
> > 2. Kernel didn't compiled with your patch.
> > 3. I simply missed it. If you believe this is the case please warn me.
> > 
> > Please help me to get these patches reviewed and queued by Daniel.
> > 
> > Also, please let me know if you have further ideas how to improve this process.
> > 
> > Thanks in advance,
> > Rodrigo.
> > 
> > 
> > Ben Widawsky (1):
> >   drm/i915: Use the real cpu max frequency for ring scaling
> > 
> > Chris Wilson (1):
> >   drm/i915: Asynchronously perform the set-base for a simple modeset
> > 
> > Daniel Vetter (1):
> >   drm/i915: check that the i965g/gm 4G limit is really obeyed
> > 
> > Paulo Zanoni (1):
> >   drm/i915: implement another plane WM workaround for HSW
> > 
> >  drivers/gpu/drm/i915/i915_gem.c      |  3 +++
> >  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++----
> >  drivers/gpu/drm/i915/intel_pm.c      | 17 +++++++++++------
> >  3 files changed, 30 insertions(+), 10 deletions(-)
> > 
> 
> Hi Rodrigo. Thanks for doing this. Could I make a small request - can
> you please CC all the relevant people when you send these mails?

Good idea. It's probably simplest to add the Reviewer: also as Cc: in the
patch before sending out the new pile.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 0/4] drm-intel-collector - review request

[Paulo Zanoni]

2013/10/7 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
>
> This is another drm-intel-collector push for review:
> http://cgit.freedesktop.org/~vivijim/drm-intel/log/?h=drm-intel-collector
>
> Here goes the list in order for better reviewers assignment:
>
> Patch     drm/i915: check that the i965g/gm 4G limit is really obeyed - Reviewer: Damien Lespiau <damien.lespiau@intel.com>
> Patch     drm/i915: Asynchronously perform the set-base for a simple modeset - Reviewer: I would prefer Ville review this since he had concerns about sorround code. switch to fbcon is much faster with this patch so I let it here and will try to review by myself if I heard nothing else until next update.
> Patch     drm/i915: implement another plane WM workaround for HSW - Reviewer: If no body else volunteer I'll do this review until next update.
> Patch     drm/i915: Use the real cpu max frequency for ring scaling - Reviewed by Chris.
>
> Overall Process:
>
> drm-intel-collector - review request
>  1. Daniel pushs drm-intel-testing (every 2 weeks)
>  2. I rebase drm-intel-collector onto drm-intel-testing
>  3. Add Reviewer: tag with voluntered reviewers. If you don't believe you should be assigned on a particular patch please don't get mad just tell you wont review or volunteer someone else.
>  4. I resubmit remaining patches for review without picking any new (drm-intel-collector - review request)
>
> drm-intel-collector - updated
>  5. One week later I collect all simple (1-2) patches that wasn't yet reviewed and not queued by Daniel from one testing update until another.
>  6. Request automated QA's PRTS automated i-g-t tests comparing drm-intel-testing x drm-intel-collector
>  7. If tests are ok I send the update notification or the patches as a series to intel-gfx mailing list for better tracking and to be reviewed. (drm-intel-collector - updated)
>  8. Let me know volunteers for review new patches and also let me know if I've picked any patch that I shouldn't.
>
> There are some reasons that some patches can be left behind:
> 1. Your patch didn't applied cleanly and I couldn't easily solve the conflicts.
> 2. Kernel didn't compiled with your patch.
> 3. I simply missed it. If you believe this is the case please warn me.
>
> Please help me to get these patches reviewed and queued by Daniel.
>
> Also, please let me know if you have further ideas how to improve this process.
>
> Thanks in advance,
> Rodrigo.
>
>
> Ben Widawsky (1):
>   drm/i915: Use the real cpu max frequency for ring scaling
>
> Chris Wilson (1):
>   drm/i915: Asynchronously perform the set-base for a simple modeset
>
> Daniel Vetter (1):
>   drm/i915: check that the i965g/gm 4G limit is really obeyed
>
> Paulo Zanoni (1):
>   drm/i915: implement another plane WM workaround for HSW

Please just discard this one, it was replaced by other patches from
Ville, Jani and Paulo.



>
>  drivers/gpu/drm/i915/i915_gem.c      |  3 +++
>  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++----
>  drivers/gpu/drm/i915/intel_pm.c      | 17 +++++++++++------
>  3 files changed, 30 insertions(+), 10 deletions(-)
>
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH 0/4] drm-intel-collector - review request

[Rodrigo Vivi]

Thanks Paulo.

Thanks for the suggestion Ben, will do this on next round!

On Tue, Oct 8, 2013 at 10:57 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
> 2013/10/7 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
>>
>> This is another drm-intel-collector push for review:
>> http://cgit.freedesktop.org/~vivijim/drm-intel/log/?h=drm-intel-collector
>>
>> Here goes the list in order for better reviewers assignment:
>>
>> Patch     drm/i915: check that the i965g/gm 4G limit is really obeyed - Reviewer: Damien Lespiau <damien.lespiau@intel.com>
>> Patch     drm/i915: Asynchronously perform the set-base for a simple modeset - Reviewer: I would prefer Ville review this since he had concerns about sorround code. switch to fbcon is much faster with this patch so I let it here and will try to review by myself if I heard nothing else until next update.
>> Patch     drm/i915: implement another plane WM workaround for HSW - Reviewer: If no body else volunteer I'll do this review until next update.
>> Patch     drm/i915: Use the real cpu max frequency for ring scaling - Reviewed by Chris.
>>
>> Overall Process:
>>
>> drm-intel-collector - review request
>>  1. Daniel pushs drm-intel-testing (every 2 weeks)
>>  2. I rebase drm-intel-collector onto drm-intel-testing
>>  3. Add Reviewer: tag with voluntered reviewers. If you don't believe you should be assigned on a particular patch please don't get mad just tell you wont review or volunteer someone else.
>>  4. I resubmit remaining patches for review without picking any new (drm-intel-collector - review request)
>>
>> drm-intel-collector - updated
>>  5. One week later I collect all simple (1-2) patches that wasn't yet reviewed and not queued by Daniel from one testing update until another.
>>  6. Request automated QA's PRTS automated i-g-t tests comparing drm-intel-testing x drm-intel-collector
>>  7. If tests are ok I send the update notification or the patches as a series to intel-gfx mailing list for better tracking and to be reviewed. (drm-intel-collector - updated)
>>  8. Let me know volunteers for review new patches and also let me know if I've picked any patch that I shouldn't.
>>
>> There are some reasons that some patches can be left behind:
>> 1. Your patch didn't applied cleanly and I couldn't easily solve the conflicts.
>> 2. Kernel didn't compiled with your patch.
>> 3. I simply missed it. If you believe this is the case please warn me.
>>
>> Please help me to get these patches reviewed and queued by Daniel.
>>
>> Also, please let me know if you have further ideas how to improve this process.
>>
>> Thanks in advance,
>> Rodrigo.
>>
>>
>> Ben Widawsky (1):
>>   drm/i915: Use the real cpu max frequency for ring scaling
>>
>> Chris Wilson (1):
>>   drm/i915: Asynchronously perform the set-base for a simple modeset
>>
>> Daniel Vetter (1):
>>   drm/i915: check that the i965g/gm 4G limit is really obeyed
>>
>> Paulo Zanoni (1):
>>   drm/i915: implement another plane WM workaround for HSW
>
> Please just discard this one, it was replaced by other patches from
> Ville, Jani and Paulo.
>
>
>
>>
>>  drivers/gpu/drm/i915/i915_gem.c      |  3 +++
>>  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++----
>>  drivers/gpu/drm/i915/intel_pm.c      | 17 +++++++++++------
>>  3 files changed, 30 insertions(+), 10 deletions(-)
>>
>> --
>> 1.8.1.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br