[PATCH 0/5] drm-intel-collector - update

[PATCH 0/5] drm-intel-collector - update

[Rodrigo Vivi]

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

Here goes the update list in order for better reviewers assignment:

Patch     drm/i915: Asynchronously perform the set-base for a simple modeset - Reviewed
Patch     drm/i915: Initialise min/max frequencies before updating RPS registers - Reviewer: Ville Syrjälä <ville.syrjala@linux.intel.com>
Patch     drm/i915: Fix gen3/4 vblank counter wraparound - Reviewer:
Patch     drm/i915: Use frame counter for intel_wait_for_vblank() on CTG - Reviewer:
Patch     drm/i915: Require HW contexts (when possible) - Reviewer:

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: Require HW contexts (when possible)

Chris Wilson (2):
  drm/i915: Asynchronously perform the set-base for a simple modeset
  drm/i915: Initialise min/max frequencies before updating RPS registers

Ville Syrjälä (2):
  drm/i915: Fix gen3/4 vblank counter wraparound
  drm/i915: Use frame counter for intel_wait_for_vblank() on CTG

 drivers/gpu/drm/i915/i915_debugfs.c     |  2 +-
 drivers/gpu/drm/i915/i915_drv.c         |  3 ---
 drivers/gpu/drm/i915/i915_drv.h         |  3 +--
 drivers/gpu/drm/i915/i915_gem.c         |  8 +++++-
 drivers/gpu/drm/i915/i915_gem_context.c | 43 +++++++++++++++------------------
 drivers/gpu/drm/i915/i915_irq.c         |  2 +-
 drivers/gpu/drm/i915/i915_sysfs.c       | 22 ++++++++---------
 drivers/gpu/drm/i915/intel_display.c    | 19 +++++++++------
 drivers/gpu/drm/i915/intel_pm.c         | 19 ++++-----------
 9 files changed, 57 insertions(+), 64 deletions(-)

-- 
1.8.3.1

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

[PATCH 1/5] 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>
Reviewed-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 f34252d..64390f3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9687,10 +9687,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.3.1

[PATCH 2/5] drm/i915: Initialise min/max frequencies before updating RPS registers

[Rodrigo Vivi]

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

The RPS register writing routines use the current value of min/max to
set certain limits and interrupt gating. If we set those afterwards, we
risk setting up the hw incorrectly and losing power management events,
and worse, trigger some internal assertions.

Reorder the calling sequences to be correct, and remove the then
unrequired clamping from inside set_rps(). And for a bonus, fix the bug
of calling gen6_set_rps() from Valleyview.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Reviewer: Ville Syrjälä <ville.syrjala@linux.intel.com>
CC: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  2 +-
 drivers/gpu/drm/i915/i915_sysfs.c   | 16 ++++++++--------
 drivers/gpu/drm/i915/intel_pm.c     | 19 +++++--------------
 3 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 7008aac..5b28602 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2758,7 +2758,7 @@ i915_max_freq_set(void *data, u64 val)
 	if (IS_VALLEYVIEW(dev)) {
 		val = vlv_freq_opcode(dev_priv->mem_freq, val);
 		dev_priv->rps.max_delay = val;
-		gen6_set_rps(dev, val);
+		valleyview_set_rps(dev, val);
 	} else {
 		do_div(val, GT_FREQUENCY_MULTIPLIER);
 		dev_priv->rps.max_delay = val;
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index cef38fd..46291c4 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -342,15 +342,15 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
 		DRM_DEBUG("User requested overclocking to %d\n",
 			  val * GT_FREQUENCY_MULTIPLIER);
 
+	dev_priv->rps.max_delay = val;
+
 	if (dev_priv->rps.cur_delay > val) {
-		if (IS_VALLEYVIEW(dev_priv->dev))
-			valleyview_set_rps(dev_priv->dev, val);
+		if (IS_VALLEYVIEW(dev))
+			valleyview_set_rps(dev, val);
 		else
-			gen6_set_rps(dev_priv->dev, val);
+			gen6_set_rps(dev, val);
 	}
 
-	dev_priv->rps.max_delay = val;
-
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
 	return count;
@@ -411,15 +411,15 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 		return -EINVAL;
 	}
 
+	dev_priv->rps.min_delay = val;
+
 	if (dev_priv->rps.cur_delay < val) {
 		if (IS_VALLEYVIEW(dev))
 			valleyview_set_rps(dev, val);
 		else
-			gen6_set_rps(dev_priv->dev, val);
+			gen6_set_rps(dev, val);
 	}
 
-	dev_priv->rps.min_delay = val;
-
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
 	return count;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 09ac9e7..830865e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3414,26 +3414,19 @@ static void ironlake_disable_drps(struct drm_device *dev)
  * ourselves, instead of doing a rmw cycle (which might result in us clearing
  * all limits and the gpu stuck at whatever frequency it is at atm).
  */
-static u32 gen6_rps_limits(struct drm_i915_private *dev_priv, u8 *val)
+static u32 gen6_rps_limits(struct drm_i915_private *dev_priv, u8 val)
 {
 	u32 limits;
 
-	limits = 0;
-
-	if (*val >= dev_priv->rps.max_delay)
-		*val = dev_priv->rps.max_delay;
-	limits |= dev_priv->rps.max_delay << 24;
-
 	/* Only set the down limit when we've reached the lowest level to avoid
 	 * getting more interrupts, otherwise leave this clear. This prevents a
 	 * race in the hw when coming out of rc6: There's a tiny window where
 	 * the hw runs at the minimal clock before selecting the desired
 	 * frequency, if the down threshold expires in that window we will not
 	 * receive a down interrupt. */
-	if (*val <= dev_priv->rps.min_delay) {
-		*val = dev_priv->rps.min_delay;
+	limits = dev_priv->rps.max_delay << 24;
+	if (val <= dev_priv->rps.min_delay)
 		limits |= dev_priv->rps.min_delay << 16;
-	}
 
 	return limits;
 }
@@ -3533,7 +3526,6 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
 void gen6_set_rps(struct drm_device *dev, u8 val)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 limits = gen6_rps_limits(dev_priv, &val);
 
 	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
 	WARN_ON(val > dev_priv->rps.max_delay);
@@ -3556,7 +3548,8 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
 	/* Make sure we continue to get interrupts
 	 * until we hit the minimum or maximum frequencies.
 	 */
-	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, limits);
+	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS,
+		   gen6_rps_limits(dev_priv, val));
 
 	POSTING_READ(GEN6_RPNSWREQ);
 
@@ -3620,8 +3613,6 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	gen6_rps_limits(dev_priv, &val);
-
 	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
 	WARN_ON(val > dev_priv->rps.max_delay);
 	WARN_ON(val < dev_priv->rps.min_delay);
-- 
1.8.3.1

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

Re: [PATCH 2/5] drm/i915: Initialise min/max frequencies before updating RPS registers

[Ville Syrjälä]

On Wed, Nov 06, 2013 at 01:56:26PM -0200, Rodrigo Vivi wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> The RPS register writing routines use the current value of min/max to
> set certain limits and interrupt gating. If we set those afterwards, we
> risk setting up the hw incorrectly and losing power management events,
> and worse, trigger some internal assertions.
> 
> Reorder the calling sequences to be correct, and remove the then
> unrequired clamping from inside set_rps(). And for a bonus, fix the bug
> of calling gen6_set_rps() from Valleyview.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> Reviewer: Ville Syrjälä <ville.syrjala@linux.intel.com>
> CC: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |  2 +-
>  drivers/gpu/drm/i915/i915_sysfs.c   | 16 ++++++++--------
>  drivers/gpu/drm/i915/intel_pm.c     | 19 +++++--------------
>  3 files changed, 14 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 7008aac..5b28602 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2758,7 +2758,7 @@ i915_max_freq_set(void *data, u64 val)
>  	if (IS_VALLEYVIEW(dev)) {
>  		val = vlv_freq_opcode(dev_priv->mem_freq, val);
>  		dev_priv->rps.max_delay = val;
> -		gen6_set_rps(dev, val);
> +		valleyview_set_rps(dev, val);
>  	} else {
>  		do_div(val, GT_FREQUENCY_MULTIPLIER);
>  		dev_priv->rps.max_delay = val;
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index cef38fd..46291c4 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -342,15 +342,15 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
>  		DRM_DEBUG("User requested overclocking to %d\n",
>  			  val * GT_FREQUENCY_MULTIPLIER);
>  
> +	dev_priv->rps.max_delay = val;
> +
>  	if (dev_priv->rps.cur_delay > val) {
> -		if (IS_VALLEYVIEW(dev_priv->dev))
> -			valleyview_set_rps(dev_priv->dev, val);
> +		if (IS_VALLEYVIEW(dev))
> +			valleyview_set_rps(dev, val);
>  		else
> -			gen6_set_rps(dev_priv->dev, val);
> +			gen6_set_rps(dev, val);
>  	}
>  
> -	dev_priv->rps.max_delay = val;
> -
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
>  	return count;
> @@ -411,15 +411,15 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
>  		return -EINVAL;
>  	}
>  
> +	dev_priv->rps.min_delay = val;
> +
>  	if (dev_priv->rps.cur_delay < val) {
>  		if (IS_VALLEYVIEW(dev))
>  			valleyview_set_rps(dev, val);
>  		else
> -			gen6_set_rps(dev_priv->dev, val);
> +			gen6_set_rps(dev, val);
>  	}
>  
> -	dev_priv->rps.min_delay = val;
> -
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
>  	return count;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 09ac9e7..830865e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3414,26 +3414,19 @@ static void ironlake_disable_drps(struct drm_device *dev)
>   * ourselves, instead of doing a rmw cycle (which might result in us clearing
>   * all limits and the gpu stuck at whatever frequency it is at atm).
>   */
> -static u32 gen6_rps_limits(struct drm_i915_private *dev_priv, u8 *val)
> +static u32 gen6_rps_limits(struct drm_i915_private *dev_priv, u8 val)
>  {
>  	u32 limits;
>  
> -	limits = 0;
> -
> -	if (*val >= dev_priv->rps.max_delay)
> -		*val = dev_priv->rps.max_delay;
> -	limits |= dev_priv->rps.max_delay << 24;
> -
>  	/* Only set the down limit when we've reached the lowest level to avoid
>  	 * getting more interrupts, otherwise leave this clear. This prevents a
>  	 * race in the hw when coming out of rc6: There's a tiny window where
>  	 * the hw runs at the minimal clock before selecting the desired
>  	 * frequency, if the down threshold expires in that window we will not
>  	 * receive a down interrupt. */
> -	if (*val <= dev_priv->rps.min_delay) {
> -		*val = dev_priv->rps.min_delay;
> +	limits = dev_priv->rps.max_delay << 24;
> +	if (val <= dev_priv->rps.min_delay)
>  		limits |= dev_priv->rps.min_delay << 16;
> -	}
>  
>  	return limits;
>  }
> @@ -3533,7 +3526,6 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
>  void gen6_set_rps(struct drm_device *dev, u8 val)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	u32 limits = gen6_rps_limits(dev_priv, &val);
>  
>  	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>  	WARN_ON(val > dev_priv->rps.max_delay);
> @@ -3556,7 +3548,8 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
>  	/* Make sure we continue to get interrupts
>  	 * until we hit the minimum or maximum frequencies.
>  	 */
> -	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, limits);
> +	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS,
> +		   gen6_rps_limits(dev_priv, val));
>  
>  	POSTING_READ(GEN6_RPNSWREQ);
>  
> @@ -3620,8 +3613,6 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	gen6_rps_limits(dev_priv, &val);
> -
>  	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>  	WARN_ON(val > dev_priv->rps.max_delay);
>  	WARN_ON(val < dev_priv->rps.min_delay);
> -- 
> 1.8.3.1

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 2/5] drm/i915: Initialise min/max frequencies before updating RPS registers

[Daniel Vetter]

On Thu, Nov 07, 2013 at 07:43:48PM +0200, Ville Syrjälä wrote:
> On Wed, Nov 06, 2013 at 01:56:26PM -0200, Rodrigo Vivi wrote:
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > The RPS register writing routines use the current value of min/max to
> > set certain limits and interrupt gating. If we set those afterwards, we
> > risk setting up the hw incorrectly and losing power management events,
> > and worse, trigger some internal assertions.
> > 
> > Reorder the calling sequences to be correct, and remove the then
> > unrequired clamping from inside set_rps(). And for a bonus, fix the bug
> > of calling gen6_set_rps() from Valleyview.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> > Reviewer: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > CC: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c |  2 +-
> >  drivers/gpu/drm/i915/i915_sysfs.c   | 16 ++++++++--------
> >  drivers/gpu/drm/i915/intel_pm.c     | 19 +++++--------------
> >  3 files changed, 14 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 7008aac..5b28602 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2758,7 +2758,7 @@ i915_max_freq_set(void *data, u64 val)
> >  	if (IS_VALLEYVIEW(dev)) {
> >  		val = vlv_freq_opcode(dev_priv->mem_freq, val);
> >  		dev_priv->rps.max_delay = val;
> > -		gen6_set_rps(dev, val);
> > +		valleyview_set_rps(dev, val);
> >  	} else {
> >  		do_div(val, GT_FREQUENCY_MULTIPLIER);
> >  		dev_priv->rps.max_delay = val;
> > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> > index cef38fd..46291c4 100644
> > --- a/drivers/gpu/drm/i915/i915_sysfs.c
> > +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> > @@ -342,15 +342,15 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
> >  		DRM_DEBUG("User requested overclocking to %d\n",
> >  			  val * GT_FREQUENCY_MULTIPLIER);
> >  
> > +	dev_priv->rps.max_delay = val;
> > +
> >  	if (dev_priv->rps.cur_delay > val) {
> > -		if (IS_VALLEYVIEW(dev_priv->dev))
> > -			valleyview_set_rps(dev_priv->dev, val);
> > +		if (IS_VALLEYVIEW(dev))
> > +			valleyview_set_rps(dev, val);
> >  		else
> > -			gen6_set_rps(dev_priv->dev, val);
> > +			gen6_set_rps(dev, val);

Extracting all these IS_VLV blocks and having one abstract entry point
would be a nice follow-up ...

Queued for -next, thanks for the patch.
-Daniel

> >  	}
> >  
> > -	dev_priv->rps.max_delay = val;
> > -
> >  	mutex_unlock(&dev_priv->rps.hw_lock);
> >  
> >  	return count;
> > @@ -411,15 +411,15 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
> >  		return -EINVAL;
> >  	}
> >  
> > +	dev_priv->rps.min_delay = val;
> > +
> >  	if (dev_priv->rps.cur_delay < val) {
> >  		if (IS_VALLEYVIEW(dev))
> >  			valleyview_set_rps(dev, val);
> >  		else
> > -			gen6_set_rps(dev_priv->dev, val);
> > +			gen6_set_rps(dev, val);
> >  	}
> >  
> > -	dev_priv->rps.min_delay = val;
> > -
> >  	mutex_unlock(&dev_priv->rps.hw_lock);
> >  
> >  	return count;
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 09ac9e7..830865e 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3414,26 +3414,19 @@ static void ironlake_disable_drps(struct drm_device *dev)
> >   * ourselves, instead of doing a rmw cycle (which might result in us clearing
> >   * all limits and the gpu stuck at whatever frequency it is at atm).
> >   */
> > -static u32 gen6_rps_limits(struct drm_i915_private *dev_priv, u8 *val)
> > +static u32 gen6_rps_limits(struct drm_i915_private *dev_priv, u8 val)
> >  {
> >  	u32 limits;
> >  
> > -	limits = 0;
> > -
> > -	if (*val >= dev_priv->rps.max_delay)
> > -		*val = dev_priv->rps.max_delay;
> > -	limits |= dev_priv->rps.max_delay << 24;
> > -
> >  	/* Only set the down limit when we've reached the lowest level to avoid
> >  	 * getting more interrupts, otherwise leave this clear. This prevents a
> >  	 * race in the hw when coming out of rc6: There's a tiny window where
> >  	 * the hw runs at the minimal clock before selecting the desired
> >  	 * frequency, if the down threshold expires in that window we will not
> >  	 * receive a down interrupt. */
> > -	if (*val <= dev_priv->rps.min_delay) {
> > -		*val = dev_priv->rps.min_delay;
> > +	limits = dev_priv->rps.max_delay << 24;
> > +	if (val <= dev_priv->rps.min_delay)
> >  		limits |= dev_priv->rps.min_delay << 16;
> > -	}
> >  
> >  	return limits;
> >  }
> > @@ -3533,7 +3526,6 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
> >  void gen6_set_rps(struct drm_device *dev, u8 val)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > -	u32 limits = gen6_rps_limits(dev_priv, &val);
> >  
> >  	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
> >  	WARN_ON(val > dev_priv->rps.max_delay);
> > @@ -3556,7 +3548,8 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
> >  	/* Make sure we continue to get interrupts
> >  	 * until we hit the minimum or maximum frequencies.
> >  	 */
> > -	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, limits);
> > +	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS,
> > +		   gen6_rps_limits(dev_priv, val));
> >  
> >  	POSTING_READ(GEN6_RPNSWREQ);
> >  
> > @@ -3620,8 +3613,6 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  
> > -	gen6_rps_limits(dev_priv, &val);
> > -
> >  	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
> >  	WARN_ON(val > dev_priv->rps.max_delay);
> >  	WARN_ON(val < dev_priv->rps.min_delay);
> > -- 
> > 1.8.3.1
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 3/5] drm/i915: Fix gen3/4 vblank counter wraparound

[Rodrigo Vivi]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

When the hardware frame counter reads 0xffffff and we're already past
vblank start, we'd return 0x1000000 as the vblank counter value. Once
we'd cross into the next frame's active portion, the vblank counter
would wrap to 0. So we're reporting two different vblank counter values
for the same frame.

Fix the problem by masking the cooked value by 0xffffff to make sure
the counter wraps already after vblank start.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2a44816..c474dac 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -583,7 +583,7 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, int pipe)
 	 * Cook up a vblank counter by also checking the pixel
 	 * counter against vblank start.
 	 */
-	return ((high1 << 8) | low) + (pixel >= vbl_start);
+	return (((high1 << 8) | low) + (pixel >= vbl_start)) & 0xffffff;
 }
 
 static u32 gm45_get_vblank_counter(struct drm_device *dev, int pipe)
-- 
1.8.3.1

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

Re: [PATCH 3/5] drm/i915: Fix gen3/4 vblank counter wraparound

[Imre Deak]

On Wed, 2013-11-06 at 13:56 -0200, Rodrigo Vivi wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> When the hardware frame counter reads 0xffffff and we're already past
> vblank start, we'd return 0x1000000 as the vblank counter value. Once
> we'd cross into the next frame's active portion, the vblank counter
> would wrap to 0. So we're reporting two different vblank counter values
> for the same frame.
> 
> Fix the problem by masking the cooked value by 0xffffff to make sure
> the counter wraps already after vblank start.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 2a44816..c474dac 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -583,7 +583,7 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, int pipe)
>  	 * Cook up a vblank counter by also checking the pixel
>  	 * counter against vblank start.
>  	 */
> -	return ((high1 << 8) | low) + (pixel >= vbl_start);
> +	return (((high1 << 8) | low) + (pixel >= vbl_start)) & 0xffffff;
>  }
>  
>  static u32 gm45_get_vblank_counter(struct drm_device *dev, int pipe)


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

Re: [PATCH 3/5] drm/i915: Fix gen3/4 vblank counter wraparound

[Daniel Vetter]

On Thu, Nov 07, 2013 at 06:16:47PM +0200, Imre Deak wrote:
> On Wed, 2013-11-06 at 13:56 -0200, Rodrigo Vivi wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > When the hardware frame counter reads 0xffffff and we're already past
> > vblank start, we'd return 0x1000000 as the vblank counter value. Once
> > we'd cross into the next frame's active portion, the vblank counter
> > would wrap to 0. So we're reporting two different vblank counter values
> > for the same frame.
> > 
> > Fix the problem by masking the cooked value by 0xffffff to make sure
> > the counter wraps already after vblank start.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> 
> Reviewed-by: Imre Deak <imre.deak@intel.com>

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

[PATCH 4/5] drm/i915: Use frame counter for intel_wait_for_vblank() on CTG

[Rodrigo Vivi]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use the same wait_for_vblank code for CTG that we use for ILK+.

Also fix the name of the frame counter register while at it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/intel_display.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 64390f3..7595d5a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -748,10 +748,10 @@ enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv,
 	return intel_crtc->config.cpu_transcoder;
 }
 
-static void ironlake_wait_for_vblank(struct drm_device *dev, int pipe)
+static void g4x_wait_for_vblank(struct drm_device *dev, int pipe)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 frame, frame_reg = PIPEFRAME(pipe);
+	u32 frame, frame_reg = PIPE_FRMCOUNT_GM45(pipe);
 
 	frame = I915_READ(frame_reg);
 
@@ -772,8 +772,8 @@ void intel_wait_for_vblank(struct drm_device *dev, int pipe)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int pipestat_reg = PIPESTAT(pipe);
 
-	if (INTEL_INFO(dev)->gen >= 5) {
-		ironlake_wait_for_vblank(dev, pipe);
+	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
+		g4x_wait_for_vblank(dev, pipe);
 		return;
 	}
 
-- 
1.8.3.1

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

Re: [PATCH 4/5] drm/i915: Use frame counter for intel_wait_for_vblank() on CTG

[Imre Deak]

On Wed, 2013-11-06 at 13:56 -0200, Rodrigo Vivi wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Use the same wait_for_vblank code for CTG that we use for ILK+.
> 
> Also fix the name of the frame counter register while at it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 64390f3..7595d5a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -748,10 +748,10 @@ enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv,
>  	return intel_crtc->config.cpu_transcoder;
>  }
>  
> -static void ironlake_wait_for_vblank(struct drm_device *dev, int pipe)
> +static void g4x_wait_for_vblank(struct drm_device *dev, int pipe)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	u32 frame, frame_reg = PIPEFRAME(pipe);
> +	u32 frame, frame_reg = PIPE_FRMCOUNT_GM45(pipe);
>  
>  	frame = I915_READ(frame_reg);
>  
> @@ -772,8 +772,8 @@ void intel_wait_for_vblank(struct drm_device *dev, int pipe)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int pipestat_reg = PIPESTAT(pipe);
>  
> -	if (INTEL_INFO(dev)->gen >= 5) {
> -		ironlake_wait_for_vblank(dev, pipe);
> +	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
> +		g4x_wait_for_vblank(dev, pipe);
>  		return;
>  	}
>  


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

[PATCH 5/5] drm/i915: Require HW contexts (when possible)

[Rodrigo Vivi]

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

v2: Fixed the botched locking on init_hw failure in i915_reset (Ville)
Call cleanup_ringbuffer on failed context create in init_hw (Ville)

v3: Add dev argument ti clean_ringbuffer

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_drv.c         |  3 ---
 drivers/gpu/drm/i915/i915_drv.h         |  3 +--
 drivers/gpu/drm/i915/i915_gem.c         |  8 +++++-
 drivers/gpu/drm/i915/i915_gem_context.c | 43 +++++++++++++++------------------
 drivers/gpu/drm/i915/i915_sysfs.c       |  6 ++---
 5 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a0804fa..6544757 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -755,12 +755,9 @@ int i915_reset(struct drm_device *dev)
 	 */
 	if (drm_core_check_feature(dev, DRIVER_MODESET) ||
 			!dev_priv->ums.mm_suspended) {
-		bool hw_contexts_disabled = dev_priv->hw_contexts_disabled;
 		dev_priv->ums.mm_suspended = 0;
 
 		ret = i915_gem_init_hw(dev);
-		if (!hw_contexts_disabled && dev_priv->hw_contexts_disabled)
-			DRM_ERROR("HW contexts didn't survive reset\n");
 		mutex_unlock(&dev->struct_mutex);
 		if (ret) {
 			DRM_ERROR("Failed hw init on reset %d\n", ret);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b12d942..e91d8fc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1447,7 +1447,6 @@ typedef struct drm_i915_private {
 	struct drm_property *broadcast_rgb_property;
 	struct drm_property *force_audio_property;
 
-	bool hw_contexts_disabled;
 	uint32_t hw_context_size;
 	struct list_head context_list;
 
@@ -2151,7 +2150,7 @@ i915_gem_obj_ggtt_pin(struct drm_i915_gem_object *obj,
 }
 
 /* i915_gem_context.c */
-void i915_gem_context_init(struct drm_device *dev);
+int __must_check i915_gem_context_init(struct drm_device *dev);
 void i915_gem_context_fini(struct drm_device *dev);
 void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
 int i915_switch_context(struct intel_ring_buffer *ring,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e7b39d7..bc52820 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4463,7 +4463,13 @@ i915_gem_init_hw(struct drm_device *dev)
 	 * XXX: There was some w/a described somewhere suggesting loading
 	 * contexts before PPGTT.
 	 */
-	i915_gem_context_init(dev);
+	ret = i915_gem_context_init(dev);
+	if (ret) {
+		i915_gem_cleanup_ringbuffer(dev);
+		DRM_ERROR("Context initialization failed %d\n", ret);
+		return ret;
+	}
+
 	if (dev_priv->mm.aliasing_ppgtt) {
 		ret = dev_priv->mm.aliasing_ppgtt->enable(dev);
 		if (ret) {
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index cc619c1..4625670 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -244,36 +244,34 @@ err_destroy:
 	return ret;
 }
 
-void i915_gem_context_init(struct drm_device *dev)
+int i915_gem_context_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	int ret;
 
-	if (!HAS_HW_CONTEXTS(dev)) {
-		dev_priv->hw_contexts_disabled = true;
-		DRM_DEBUG_DRIVER("Disabling HW Contexts; old hardware\n");
-		return;
-	}
+	if (!HAS_HW_CONTEXTS(dev))
+		return 0;
 
 	/* If called from reset, or thaw... we've been here already */
-	if (dev_priv->hw_contexts_disabled ||
-	    dev_priv->ring[RCS].default_context)
-		return;
+	if (dev_priv->ring[RCS].default_context)
+		return 0;
 
 	dev_priv->hw_context_size = round_up(get_context_size(dev), 4096);
 
 	if (dev_priv->hw_context_size > (1<<20)) {
-		dev_priv->hw_contexts_disabled = true;
 		DRM_DEBUG_DRIVER("Disabling HW Contexts; invalid size\n");
-		return;
+		return -E2BIG;
 	}
 
-	if (create_default_context(dev_priv)) {
-		dev_priv->hw_contexts_disabled = true;
-		DRM_DEBUG_DRIVER("Disabling HW Contexts; create failed\n");
-		return;
+	ret = create_default_context(dev_priv);
+	if (ret) {
+		DRM_DEBUG_DRIVER("Disabling HW Contexts; create failed %d\n",
+				 ret);
+		return ret;
 	}
 
 	DRM_DEBUG_DRIVER("HW context support initialized\n");
+	return 0;
 }
 
 void i915_gem_context_fini(struct drm_device *dev)
@@ -281,7 +279,7 @@ void i915_gem_context_fini(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct i915_hw_context *dctx = dev_priv->ring[RCS].default_context;
 
-	if (dev_priv->hw_contexts_disabled)
+	if (!HAS_HW_CONTEXTS(dev))
 		return;
 
 	/* The only known way to stop the gpu from accessing the hw context is
@@ -324,16 +322,16 @@ i915_gem_context_get_hang_stats(struct drm_device *dev,
 				struct drm_file *file,
 				u32 id)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_file_private *file_priv = file->driver_priv;
 	struct i915_hw_context *ctx;
 
 	if (id == DEFAULT_CONTEXT_ID)
 		return &file_priv->hang_stats;
 
-	ctx = NULL;
-	if (!dev_priv->hw_contexts_disabled)
-		ctx = i915_gem_context_get(file->driver_priv, id);
+	if (!HAS_HW_CONTEXTS(dev))
+		return ERR_PTR(-ENOENT);
+
+	ctx = i915_gem_context_get(file->driver_priv, id);
 	if (ctx == NULL)
 		return ERR_PTR(-ENOENT);
 
@@ -506,7 +504,7 @@ int i915_switch_context(struct intel_ring_buffer *ring,
 	struct drm_i915_private *dev_priv = ring->dev->dev_private;
 	struct i915_hw_context *to;
 
-	if (dev_priv->hw_contexts_disabled)
+	if (!HAS_HW_CONTEXTS(ring->dev))
 		return 0;
 
 	WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
@@ -531,7 +529,6 @@ int i915_switch_context(struct intel_ring_buffer *ring,
 int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
 				  struct drm_file *file)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_context_create *args = data;
 	struct drm_i915_file_private *file_priv = file->driver_priv;
 	struct i915_hw_context *ctx;
@@ -540,7 +537,7 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
 	if (!(dev->driver->driver_features & DRIVER_GEM))
 		return -ENODEV;
 
-	if (dev_priv->hw_contexts_disabled)
+	if (!HAS_HW_CONTEXTS(dev))
 		return -ENODEV;
 
 	ret = i915_mutex_lock_interruptible(dev);
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index 46291c4..c49825d 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -183,13 +183,13 @@ i915_l3_write(struct file *filp, struct kobject *kobj,
 	int slice = (int)(uintptr_t)attr->private;
 	int ret;
 
+	if (!HAS_HW_CONTEXTS(drm_dev))
+		return -ENXIO;
+
 	ret = l3_access_valid(drm_dev, offset);
 	if (ret)
 		return ret;
 
-	if (dev_priv->hw_contexts_disabled)
-		return -ENXIO;
-
 	ret = i915_mutex_lock_interruptible(drm_dev);
 	if (ret)
 		return ret;
-- 
1.8.3.1

Re: [PATCH 5/5] drm/i915: Require HW contexts (when possible)

[Daniel Vetter]

On Wed, Nov 06, 2013 at 01:56:29PM -0200, Rodrigo Vivi wrote:
> From: Ben Widawsky <benjamin.widawsky@intel.com>
> 
> v2: Fixed the botched locking on init_hw failure in i915_reset (Ville)
> Call cleanup_ringbuffer on failed context create in init_hw (Ville)
> 
> v3: Add dev argument ti clean_ringbuffer
> 
> Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> 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