Re: [Intel-gfx] [PATCH 2/9] drm/i915: Reduce clutter by using the local plane pointer

[Jesse Barnes]

On 03/10/2015 04:15 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> No need to go dig throguh intel_crtc->base.cursor when we already have
> the same thing as 'plane' local variable.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  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 1d5107e..0da3abf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12299,7 +12299,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
>  
>  finish:
>   if (intel_crtc->active) {
> - if (intel_crtc->base.cursor->state->crtc_w != 
> state->base.crtc_w)
> + if (plane->state->crtc_w != state->base.crtc_w)
>   intel_crtc->atomic.update_wm = true;
>  
>   intel_crtc->atomic.fb_bits |=
> 

At least it looks like we'll always get the right plane obj here. :)

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

Re: [Intel-gfx] [PATCH 0/2] drm/i915: Try to sort out ironlake_check_fdi_lanes()

[Ander Conselvan De Oliveira]

On Wed, 2015-03-11 at 18:52 +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> So these on top of Ander's latest FDI bifurcation patch [1] should hopefully
> result in something sane. Didn't test them though, but assuming Ander
> has that test somewhere maybe some of the problematic cases I identified [2]
> could be checked with these.

I run the tests I wrote here and they all passed. However, the tests
don't cover the eDP on pipe C case. Moreover, I don't have an IVB laptop
I could use for testing that case.

Ander

> [1] http://lists.freedesktop.org/archives/intel-gfx/2015-March/061802.html
> [2] http://lists.freedesktop.org/archives/intel-gfx/2015-March/061806.html
> 
> Ville Syrjälä (2):
>   drm/i915: Rewrite some some of the FDI lane checks
>   drm/i915: Rewrite IVB FDI bifurcation conflict checks
> 
>  drivers/gpu/drm/i915/intel_display.c | 40 
> ++--
>  1 file changed, 20 insertions(+), 20 deletions(-)
> 


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

Re: [Intel-gfx] [PATCH 3/9] drm/i915: Use plane->state->fb instead of plane->fb in intel_plane_restore()

[Daniel Vetter]

On Wed, Mar 11, 2015 at 10:41:30AM +0100, Daniel Vetter wrote:
> On Tue, Mar 10, 2015 at 07:48:39PM +0200, Ville Syrjälä wrote:
> > On Tue, Mar 10, 2015 at 10:01:47AM -0700, Matt Roper wrote:
> > > On Tue, Mar 10, 2015 at 01:15:23PM +0200, ville.syrj...@linux.intel.com 
> > > wrote:
> > > > From: Ville Syrjälä 
> > > > 
> > > > plane->fb is not as reliable as plane->state->fb so let's convert
> > > > intel_plane_restore() over the the new way of thinking as well.
> > > > 
> > > > Cc: Matt Roper 
> > > > Signed-off-by: Ville Syrjälä 
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_sprite.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
> > > > b/drivers/gpu/drm/i915/intel_sprite.c
> > > > index 7051da7..a828736 100644
> > > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > > @@ -1361,10 +1361,10 @@ out_unlock:
> > > >  
> > > >  int intel_plane_restore(struct drm_plane *plane)
> > > >  {
> > > > -   if (!plane->crtc || !plane->fb)
> > > > +   if (!plane->crtc || !plane->state->fb)
> > > > return 0;
> > > >  
> > > > -   return plane->funcs->update_plane(plane, plane->crtc, plane->fb,
> > > > +   return plane->funcs->update_plane(plane, plane->crtc, 
> > > > plane->state->fb,
> > > 
> > > While we're at it, should we s/plane->crtc/plane->state->crtc/ as well?
> > 
> > I tried to make that change everywhere and it blew up. But I think that
> > was simply because I changed it some .commit hook as well, and currently
> > we don't have the old state around there, so the 'crtc ? crtc : state->crtc'
> > just ended up as 'crtc' effectively and that of course didn't work as well
> > as I'd hoped ;)
> > 
> > But yeah maybe we should make that change. Would just need to pass the
> > old state to commit instead of the new state, I think.
> 
> Not sure we should be too agressive with mass-conversion. E.g. for the
> plane restore I expect that we'll do some overall atomic helper to
> snapshot/restore all the state for suspend/resume and similar cases.
> -Daniel
> > 
> > > 
> > > Otherwise, 1-3 are
> > > 
> > > Reviewed-by: Matt Roper 

Oh and: Merged first three, thanks for review&patches.
-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 9/9] drm/i915: Don't re-enable an explicitly disabled primary plane due to sprite coverage changes

[Daniel Vetter]

On Tue, Mar 10, 2015 at 01:15:29PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> When the sprite is covering the entire pipe (and color keying is not
> enabled) we currently try to automagically disable the primary plane
> which is fully covered by the sprite.
> 
> Now that .crtc_disable() will simply disable planes by setting the
> state->visible to false, the sprite code will actually end up
> re-enabling the primary after it got disabled, and then we proceed to
> turn off the pipe and are greeted with some warnings from
> assert_plane_disabled().
> 
> The code doing the automagic disable of covered planes needs to
> rewritten to do things correctly as part of the atomic update (or simply
> removed), but in the meantime add a a bit of duct tape and simply have
> the sprite code check the primary plane's state->visible before
> re-enabling it.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
> b/drivers/gpu/drm/i915/intel_sprite.c
> index a828736..7286497 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1287,7 +1287,9 @@ intel_commit_sprite_plane(struct drm_plane *plane,
>   intel_plane->obj = obj;
>  
>   if (intel_crtc->active) {
> - intel_crtc->primary_enabled = !state->hides_primary;
> + intel_crtc->primary_enabled =
> + to_intel_plane_state(crtc->primary->state)->visible &&
> + !state->hides_primary;

Can't we just nuke intel_crtc->primary_enabled with all your work to tread
state through functions correctly?
-Daniel

>  
>   if (state->visible) {
>   crtc_x = state->dst.x1;
> -- 
> 2.0.5
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH v2] drm/i915: Modifying RC6 Promotion timer for Media workloads.

[Deepak S]

On Friday 06 March 2015 10:10 PM, Daniel Vetter wrote:

On Thu, Mar 05, 2015 at 09:27:59PM +0530, deepa...@linux.intel.com wrote:

From: Deepak S 

In normal cases, RC6 promotion timer is 1700us/500us. This will
result in more time spent in C1 state. For more residency in
C6 in case of media workloads, this is changed to 250us.
Not doing this for 3D workloads as too many C6-C0
transition delays can result in performance impact.

v2: Extend GPU busy & idle detection framework for rc6 Promotion
timer changes (Chris)

Signed-off-by: Deepak S 

I've thougth Chris' idea was to put this into the gen6_rps_boost/idle
functions? You could check from within them I think for whether the vcs is
still busy ... One more comment below.
-Daniel


Hi Daniel,

gen6_rps_boost/idle will be called only for RCS right? Also we get 
gen6_rps_boost during  __wait_request
But we want to program promotion timer when we add request to VCS to apply the 
value immediately.

Thanks
Deepak


---
  drivers/gpu/drm/i915/i915_gem.c  | 10 +-
  drivers/gpu/drm/i915/intel_display.c |  3 ++-
  drivers/gpu/drm/i915/intel_drv.h |  2 ++
  drivers/gpu/drm/i915/intel_pm.c  | 27 +++
  4 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3831cc0..85f8aa6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2428,7 +2428,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
struct drm_i915_gem_request *request;
struct intel_ringbuffer *ringbuf;
u32 request_start;
-   int ret;
+   int ret, was_empty;
  
  	request = ring->outstanding_lazy_request;

if (WARN_ON(request == NULL))
@@ -2495,6 +2495,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
}
  
  	request->emitted_jiffies = jiffies;

+   was_empty = list_empty(&ring->request_list);
list_add_tail(&request->list, &ring->request_list);
request->file_priv = NULL;
  
@@ -2519,6 +2520,10 @@ int __i915_add_request(struct intel_engine_cs *ring,

queue_delayed_work(dev_priv->wq,
   &dev_priv->mm.retire_work,
   round_jiffies_up_relative(HZ));
+
+   if ((ring->id == VCS) && was_empty)
+   vlv_media_promotion_timer_busy(dev_priv);
+
intel_mark_busy(dev_priv->dev);
  
  	return 0;

@@ -2802,6 +2807,9 @@ i915_gem_retire_requests_ring(struct intel_engine_cs 
*ring)
}
  
  	WARN_ON(i915_verify_lists(ring->dev));

+
+   if (ring->id == VCS && list_empty(&ring->request_list))
+   vlv_media_promotion_timer_idle(dev_priv);
  }
  
  bool

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 597c10b..5d121b4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9172,8 +9172,9 @@ void intel_mark_idle(struct drm_device *dev)
intel_decrease_pllclock(crtc);
}
  
-	if (INTEL_INFO(dev)->gen >= 6)

+   if (INTEL_INFO(dev)->gen >= 6) {
gen6_rps_idle(dev->dev_private);
+   }

Uncessary hunk. And a bikeshed: I think generally if we name something
vlv_ we put the platform checks outside of the function. Or have some
other guarantee in place to make sure it's only called on the right
platforms. Otherwise we generally pick an intel_ prefix.


Thanks Daniel. I will create intel_ prefix, we might need to extend this for 
future platforms.

  
  out:

intel_runtime_pm_put(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2a6ec4b..f1a90b8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1233,6 +1233,8 @@ void ironlake_teardown_rc6(struct drm_device *dev);
  void gen6_update_ring_freq(struct drm_device *dev);
  void gen6_rps_idle(struct drm_i915_private *dev_priv);
  void gen6_rps_boost(struct drm_i915_private *dev_priv);
+void vlv_media_promotion_timer_idle(struct drm_i915_private *dev_priv);
+void vlv_media_promotion_timer_busy(struct drm_i915_private *dev_priv);
  void ilk_wm_get_hw_state(struct drm_device *dev);
  void skl_wm_get_hw_state(struct drm_device *dev);
  void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e710b43..d23b60a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3961,6 +3961,33 @@ void gen6_rps_boost(struct drm_i915_private *dev_priv)
mutex_unlock(&dev_priv->rps.hw_lock);
  }
  
+void vlv_media_promotion_timer_idle(struct drm_i915_private *dev_priv)

+{
+   struct drm_device *dev = dev_priv->dev;
+
+   if (!IS_VALLEYVIEW(dev))
+   return;
+
+   if (IS_CHERRYVIEW(dev_priv->dev)) {
+   /* TO threshold set to 500 us ( 0x186 * 1.28 us) */
+   I915_WRITE(GEN6_RC6_THRESH

Re: [Intel-gfx] [PATCH 2/2] drm/i915: Rewrite IVB FDI bifurcation conflict checks

[Ander Conselvan De Oliveira]

(for the series)
Reviewed-by: Ander Conselvan de Oliveira 

On Wed, 2015-03-11 at 18:52 +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Ignore the current state of the pipe and just check crtc_state->enable
> and the number of FDI lanes required. This means we don't accidentally
> mistake the FDI lanes as being available of one of the pipes just
> happens to be disabled at the time of the check. Also we no longer
> consider pipe C to require FDI lanes when it's driving the eDP
> transcoder.
> 
> We also take the opportunity to make the code a bit nicer looking by
> hiding the ugly bits in the new pipe_required_fdi_lanes() function.
> 
> Cc: Ander Conselvan de Oliveira 
> Cc: Daniel Vetter 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 29 +++--
>  1 file changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 242a8a7..72e9816 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3152,12 +3152,6 @@ static void intel_fdi_normal_train(struct drm_crtc 
> *crtc)
>  FDI_FE_ERRC_ENABLE);
>  }
>  
> -static bool pipe_has_enabled_pch(struct intel_crtc *crtc)
> -{
> - return crtc->base.state->enable && crtc->active &&
> - crtc->config->has_pch_encoder;
> -}
> -
>  /* The FDI link training functions for ILK/Ibexpeak. */
>  static void ironlake_fdi_link_train(struct drm_crtc *crtc)
>  {
> @@ -5548,13 +5542,21 @@ bool intel_connector_get_hw_state(struct 
> intel_connector *connector)
>   return encoder->get_hw_state(encoder, &pipe);
>  }
>  
> +static int pipe_required_fdi_lanes(struct drm_device *dev, enum pipe pipe)
> +{
> + struct intel_crtc *crtc =
> + to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> +
> + if (crtc->base.state->enable &&
> + crtc->config->has_pch_encoder)
> + return crtc->config->fdi_lanes;
> +
> + return 0;
> +}
> +
>  static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
>struct intel_crtc_state *pipe_config)
>  {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *pipe_B_crtc =
> - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
> -
>   DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
> pipe_name(pipe), pipe_config->fdi_lanes);
>   if (pipe_config->fdi_lanes > 4) {
> @@ -5581,8 +5583,8 @@ static bool ironlake_check_fdi_lanes(struct drm_device 
> *dev, enum pipe pipe,
>   case PIPE_A:
>   return true;
>   case PIPE_B:
> - if (dev_priv->pipe_to_crtc_mapping[PIPE_C]->enabled &&
> - pipe_config->fdi_lanes > 2) {
> + if (pipe_config->fdi_lanes > 2 &&
> + pipe_required_fdi_lanes(dev, PIPE_C) > 0) {
>   DRM_DEBUG_KMS("invalid shared fdi lane config on pipe 
> %c: %i lanes\n",
> pipe_name(pipe), pipe_config->fdi_lanes);
>   return false;
> @@ -5594,8 +5596,7 @@ static bool ironlake_check_fdi_lanes(struct drm_device 
> *dev, enum pipe pipe,
> pipe_name(pipe), pipe_config->fdi_lanes);
>   return false;
>   }
> - if (pipe_has_enabled_pch(pipe_B_crtc) &&
> - pipe_B_crtc->config->fdi_lanes > 2) {
> + if (pipe_required_fdi_lanes(dev, PIPE_B) > 2) {
>   DRM_DEBUG_KMS("fdi link B uses too many lanes to enable 
> link C\n");
>   return false;
>   }


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

Re: [Intel-gfx] [PATCH 5/9] drm/i915: Pass primary plane size to .update_primary_plane()

[Matt Roper]

On Tue, Mar 10, 2015 at 01:15:25PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> In preparation to movable/resizable primary planes pass the clipped
> plane size to .update_primary_plane().

Personally I feel like it would make more sense to just completely kill
off .update_primary_plane() now rather than trying to evolve it.  We
already have an intel_plane->update_plane() function pointer which is
never set or called for non-sprites at the moment.  We could unify the
handling of low-level plane programming by just using that function
pointer for primary planes as well.

I wouldn't mind also seeing the name of that low-level entrypoint
renamed to something like 'update_hw_plane' to avoid confusion with the
drm_plane entrypoint...


Matt

> 
> Cc: Sonika Jindal 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  3 +-
>  drivers/gpu/drm/i915/intel_display.c | 63 
> 
>  2 files changed, 38 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b16c0a7..e99eef0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -578,7 +578,8 @@ struct drm_i915_display_funcs {
> uint32_t flags);
>   void (*update_primary_plane)(struct drm_crtc *crtc,
>struct drm_framebuffer *fb,
> -  int x, int y);
> +  int x, int y,
> +  int crtc_w, int crtc_h);
>   void (*hpd_irq_setup)(struct drm_device *dev);
>   /* clock updates for mode set */
>   /* cursor updates */
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index fdc83f1..1a789f0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2482,7 +2482,8 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc,
>  
>  static void i9xx_update_primary_plane(struct drm_crtc *crtc,
> struct drm_framebuffer *fb,
> -   int x, int y)
> +   int x, int y,
> +   int crtc_w, int crtc_h)
>  {
>   struct drm_device *dev = crtc->dev;
>   struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -2517,20 +2518,16 @@ static void i9xx_update_primary_plane(struct drm_crtc 
> *crtc,
>   if (INTEL_INFO(dev)->gen < 4) {
>   if (intel_crtc->pipe == PIPE_B)
>   dspcntr |= DISPPLANE_SEL_PIPE_B;
> -
> - /* pipesrc and dspsize control the size that is scaled from,
> -  * which should always be the user's requested size.
> -  */
> - I915_WRITE(DSPSIZE(plane),
> -((intel_crtc->config->pipe_src_h - 1) << 16) |
> -(intel_crtc->config->pipe_src_w - 1));
> + I915_WRITE(DSPSIZE(plane), ((crtc_h - 1) << 16) | (crtc_w - 1));
>   I915_WRITE(DSPPOS(plane), 0);
>   } else if (IS_CHERRYVIEW(dev) && plane == PLANE_B) {
> - I915_WRITE(PRIMSIZE(plane),
> -((intel_crtc->config->pipe_src_h - 1) << 16) |
> -(intel_crtc->config->pipe_src_w - 1));
> + I915_WRITE(PRIMSIZE(plane), ((crtc_h - 1) << 16) | (crtc_w - 
> 1));
>   I915_WRITE(PRIMPOS(plane), 0);
>   I915_WRITE(PRIMCNSTALPHA(plane), 0);
> + } else {
> + WARN_ONCE(crtc_w != intel_crtc->config->pipe_src_w ||
> +   crtc_h != intel_crtc->config->pipe_src_h,
> +   "primary plane size doesn't match pipe size\n");
>   }
>  
>   switch (fb->pixel_format) {
> @@ -2586,14 +2583,14 @@ static void i9xx_update_primary_plane(struct drm_crtc 
> *crtc,
>   if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180)) {
>   dspcntr |= DISPPLANE_ROTATE_180;
>  
> - x += (intel_crtc->config->pipe_src_w - 1);
> - y += (intel_crtc->config->pipe_src_h - 1);
> + x += (crtc_w - 1);
> + y += (crtc_h - 1);
>  
>   /* Finding the last pixel of the last line of the display
>   data and adding to linear_offset*/
>   linear_offset +=
> - (intel_crtc->config->pipe_src_h - 1) * fb->pitches[0] +
> - (intel_crtc->config->pipe_src_w - 1) * pixel_size;
> + (crtc_h - 1) * fb->pitches[0] +
> + (crtc_w - 1) * pixel_size;
>   }
>  
>   I915_WRITE(reg, dspcntr);
> @@ -2611,7 +2608,8 @@ static void i9xx_update_primary_plane(struct drm_crtc 
> *crtc,
>  
>  static void ironlake_update_primary_plane(struct drm_crtc *crtc,
> struct drm_framebuffer *fb,
> -   

[Intel-gfx] [PATCH] drm/i915: Remove the preliminary_hw_support shackles from CHV

[ville . syrjala]

From: Ville Syrjälä 

CHV should be in a good enough shape now, so let's drop the
.is_preliminary flag.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_drv.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 15f58d0..82f8be4 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -346,7 +346,6 @@ static const struct intel_device_info 
intel_broadwell_gt3m_info = {
 };
 
 static const struct intel_device_info intel_cherryview_info = {
-   .is_preliminary = 1,
.gen = 8, .num_pipes = 3,
.need_gfx_hws = 1, .has_hotplug = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
-- 
2.0.5

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

Re: [Intel-gfx] [PATCH 5/9] drm/i915: Pass primary plane size to .update_primary_plane()

[Ville Syrjälä]

On Tue, Mar 10, 2015 at 10:10:40AM -0700, Matt Roper wrote:
> On Tue, Mar 10, 2015 at 01:15:25PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> > 
> > In preparation to movable/resizable primary planes pass the clipped
> > plane size to .update_primary_plane().
> 
> Personally I feel like it would make more sense to just completely kill
> off .update_primary_plane() now rather than trying to evolve it.  We
> already have an intel_plane->update_plane() function pointer which is
> never set or called for non-sprites at the moment.  We could unify the
> handling of low-level plane programming by just using that function
> pointer for primary planes as well.

I want to kill it off as well, but that means either killing off
set_base_atomic() or making it use the plane commit hook. I suppose we
could hand craft a suitable plane state for it and just commit that
without any checks or anything?

> 
> I wouldn't mind also seeing the name of that low-level entrypoint
> renamed to something like 'update_hw_plane' to avoid confusion with the
> drm_plane entrypoint...
> 
> 
> Matt
> 
> > 
> > Cc: Sonika Jindal 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h  |  3 +-
> >  drivers/gpu/drm/i915/intel_display.c | 63 
> > 
> >  2 files changed, 38 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index b16c0a7..e99eef0 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -578,7 +578,8 @@ struct drm_i915_display_funcs {
> >   uint32_t flags);
> > void (*update_primary_plane)(struct drm_crtc *crtc,
> >  struct drm_framebuffer *fb,
> > -int x, int y);
> > +int x, int y,
> > +int crtc_w, int crtc_h);
> > void (*hpd_irq_setup)(struct drm_device *dev);
> > /* clock updates for mode set */
> > /* cursor updates */
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index fdc83f1..1a789f0 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2482,7 +2482,8 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc,
> >  
> >  static void i9xx_update_primary_plane(struct drm_crtc *crtc,
> >   struct drm_framebuffer *fb,
> > - int x, int y)
> > + int x, int y,
> > + int crtc_w, int crtc_h)
> >  {
> > struct drm_device *dev = crtc->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -2517,20 +2518,16 @@ static void i9xx_update_primary_plane(struct 
> > drm_crtc *crtc,
> > if (INTEL_INFO(dev)->gen < 4) {
> > if (intel_crtc->pipe == PIPE_B)
> > dspcntr |= DISPPLANE_SEL_PIPE_B;
> > -
> > -   /* pipesrc and dspsize control the size that is scaled from,
> > -* which should always be the user's requested size.
> > -*/
> > -   I915_WRITE(DSPSIZE(plane),
> > -  ((intel_crtc->config->pipe_src_h - 1) << 16) |
> > -  (intel_crtc->config->pipe_src_w - 1));
> > +   I915_WRITE(DSPSIZE(plane), ((crtc_h - 1) << 16) | (crtc_w - 1));
> > I915_WRITE(DSPPOS(plane), 0);
> > } else if (IS_CHERRYVIEW(dev) && plane == PLANE_B) {
> > -   I915_WRITE(PRIMSIZE(plane),
> > -  ((intel_crtc->config->pipe_src_h - 1) << 16) |
> > -  (intel_crtc->config->pipe_src_w - 1));
> > +   I915_WRITE(PRIMSIZE(plane), ((crtc_h - 1) << 16) | (crtc_w - 
> > 1));
> > I915_WRITE(PRIMPOS(plane), 0);
> > I915_WRITE(PRIMCNSTALPHA(plane), 0);
> > +   } else {
> > +   WARN_ONCE(crtc_w != intel_crtc->config->pipe_src_w ||
> > + crtc_h != intel_crtc->config->pipe_src_h,
> > + "primary plane size doesn't match pipe size\n");
> > }
> >  
> > switch (fb->pixel_format) {
> > @@ -2586,14 +2583,14 @@ static void i9xx_update_primary_plane(struct 
> > drm_crtc *crtc,
> > if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180)) {
> > dspcntr |= DISPPLANE_ROTATE_180;
> >  
> > -   x += (intel_crtc->config->pipe_src_w - 1);
> > -   y += (intel_crtc->config->pipe_src_h - 1);
> > +   x += (crtc_w - 1);
> > +   y += (crtc_h - 1);
> >  
> > /* Finding the last pixel of the last line of the display
> > data and adding to linear_offset*/
> > linear_offset +=
> > -   (intel_crtc->config->pipe_src_h - 1) * fb->pitches[0] +
> > -   (intel_crtc->config->pipe_src_w - 1) * pixel_size;
> > +   

Re: [Intel-gfx] [PATCH 4/9] drm/i915: Make derived plane state correct after crtc_enable

[Ville Syrjälä]

On Tue, Mar 10, 2015 at 10:01:51AM -0700, Matt Roper wrote:
> On Tue, Mar 10, 2015 at 01:15:24PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> > 
> > We need to call the plane .atomic_check() hook when enabling the crtc
> > to make sure all the derived state (eg. visible, clipped src/dst
> > coordinates) are up to date when we enable the plane. This allows us to
> > trust this derived state everywhere.
> > 
> > We also take the opportunity to rewrite the plane enable sequence to
> > perform it as a single atomic update, which is a bit closer to where we
> > want to end up. Obviously this is a bit of a hack as we can't deal with
> > errors from .atomic_check(), so we just paper over that by making sure
> > visible=false so that we don't try to enable the plane with potentially
> > garbage coordinates and whatnot.
> > 
> > We also abuse the atomic code a bit by not making another copy of the
> > plane state and just frobbing directly with the plane->state.
> > 
> > Cc: Matt Roper 
> > Cc: Sonika Jindal 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h  |   3 +
> >  drivers/gpu/drm/i915/intel_display.c | 222 
> > ++-
> >  2 files changed, 88 insertions(+), 137 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 5462cbf..b16c0a7 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -238,6 +238,9 @@ enum hpd_pin {
> >  #define for_each_intel_crtc(dev, intel_crtc) \
> > list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list, base.head)
> >  
> > +#define for_each_intel_plane(dev, intel_plane) \
> > +   list_for_each_entry(intel_plane, &dev->mode_config.plane_list, 
> > base.head)
> > +
> >  #define for_each_intel_encoder(dev, intel_encoder) \
> > list_for_each_entry(intel_encoder,  \
> > &(dev)->mode_config.encoder_list,   \
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 0da3abf..fdc83f1 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2124,66 +2124,6 @@ void intel_flush_primary_plane(struct 
> > drm_i915_private *dev_priv,
> > POSTING_READ(reg);
> >  }
> >  
> > -/**
> > - * intel_enable_primary_hw_plane - enable the primary plane on a given pipe
> > - * @plane:  plane to be enabled
> > - * @crtc: crtc for the plane
> > - *
> > - * Enable @plane on @crtc, making sure that the pipe is running first.
> > - */
> > -static void intel_enable_primary_hw_plane(struct drm_plane *plane,
> > - struct drm_crtc *crtc)
> > -{
> > -   struct drm_device *dev = plane->dev;
> > -   struct drm_i915_private *dev_priv = dev->dev_private;
> > -   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > -
> > -   /* If the pipe isn't enabled, we can't pump pixels and may hang */
> > -   assert_pipe_enabled(dev_priv, intel_crtc->pipe);
> > -
> > -   if (intel_crtc->primary_enabled)
> > -   return;
> > -
> > -   intel_crtc->primary_enabled = true;
> > -
> > -   dev_priv->display.update_primary_plane(crtc, plane->fb,
> > -  crtc->x, crtc->y);
> > -
> > -   /*
> > -* BDW signals flip done immediately if the plane
> > -* is disabled, even if the plane enable is already
> > -* armed to occur at the next vblank :(
> > -*/
> > -   if (IS_BROADWELL(dev))
> > -   intel_wait_for_vblank(dev, intel_crtc->pipe);
> > -}
> > -
> > -/**
> > - * intel_disable_primary_hw_plane - disable the primary hardware plane
> > - * @plane: plane to be disabled
> > - * @crtc: crtc for the plane
> > - *
> > - * Disable @plane on @crtc, making sure that the pipe is running first.
> > - */
> > -static void intel_disable_primary_hw_plane(struct drm_plane *plane,
> > -  struct drm_crtc *crtc)
> > -{
> > -   struct drm_device *dev = plane->dev;
> > -   struct drm_i915_private *dev_priv = dev->dev_private;
> > -   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > -
> > -   if (WARN_ON(!intel_crtc->active))
> > -   return;
> > -
> > -   if (!intel_crtc->primary_enabled)
> > -   return;
> > -
> > -   intel_crtc->primary_enabled = false;
> > -
> > -   dev_priv->display.update_primary_plane(crtc, plane->fb,
> > -  crtc->x, crtc->y);
> > -}
> > -
> >  static bool need_vtd_wa(struct drm_device *dev)
> >  {
> >  #ifdef CONFIG_INTEL_IOMMU
> > @@ -2895,7 +2835,10 @@ static void skylake_update_primary_plane(struct 
> > drm_crtc *crtc,
> > POSTING_READ(PLANE_SURF(pipe, 0));
> >  }
> >  
> > -/* Assume fb object is pinned & idle & fenced and just update base 
> > pointers */
> > +/*
> > + * Assume fb object is big enough & pinned & idle & fenced,
> > + * and just update base pointers
> > + */

Re: [Intel-gfx] [Beignet] [PATCH i-g-t 2/2] configure: Bump required libdrm version to 2.4.60

[Jeff McGee]

On Tue, Mar 10, 2015 at 01:58:52PM -0400, Rob Clark wrote:
> On Tue, Mar 10, 2015 at 12:59 PM, Jeff McGee  wrote:
> > On Tue, Mar 10, 2015 at 08:37:30AM +0100, Daniel Vetter wrote:
> >> On Mon, Mar 09, 2015 at 04:41:02PM -0700, jeff.mc...@intel.com wrote:
> >> > From: Jeff McGee 
> >> >
> >> > tests/core_getparams needs the new libdrm interfaces for
> >> > querying subslice and EU counts.
> >> >
> >> > For: VIZ-4636
> >> > Signed-off-by: Jeff McGee 
> >> > ---
> >> >  configure.ac | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/configure.ac b/configure.ac
> >> > index 16d6a2e..88a1c3d 100644
> >> > --- a/configure.ac
> >> > +++ b/configure.ac
> >> > @@ -82,7 +82,7 @@ if test "x$GCC" = "xyes"; then
> >> >  fi
> >> >  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
> >> >
> >> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.52 libdrm])
> >> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.60 libdrm])
> >>
> >> Please don't and instead copypaste the new structs/defines with a local_
> >> prefix like we do it for all the other new igt testcases. Forcing libdrm
> >> to get updated for igt all the time can get annoying fast.
> >> -Daniel
> >>
> > In this case I'm trying to exercise new API functions in libdrm which
> > wrap the GETPARAM ioctl. Would you rather me bypass the wrapper to
> > avoid requiring updated libdrm? I can do that, but it fails to test the
> > complete path that client would use.
> 
> 
> Am I missing something, or does 2.4.60 not exist yet?
> 
> That said dependency bumps for igt seem like less of an issue than
> dependency bumps for mesa..  I mean if you are using igt you are
> probably on the latest anyways..  I'm not sure why Daniel is so
> concerned about that..
> 
> (but dependency bumps to something that doesn't exist yet should
> perhaps be avoided)
> 
> BR,
> -R
> 

Hi Rob. This igt change is contigent upon my libdrm changes which
would in fact bump that version to 2.4.60 after adding an API. That
change is also posted and waiting review. I guess I should have stated
that dependency here to begin with.

http://lists.freedesktop.org/archives/intel-gfx/2015-March/061101.html

Jeff
> 
> > -Jeff
> >
> >> >  PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
> >> >  PKG_CHECK_MODULES(OVERLAY_XVLIB, [xv x11 xext dri2proto >= 2.6], 
> >> > enable_overlay_xvlib=yes, enable_overlay_xvlib=no)
> >> >  PKG_CHECK_MODULES(OVERLAY_XLIB, [cairo-xlib dri2proto >= 2.6], 
> >> > enable_overlay_xlib=yes, enable_overlay_xlib=no)
> >> > --
> >> > 2.3.0
> >> >
> >> > ___
> >> > 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
> >> ___
> >> Beignet mailing list
> >> beig...@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/beignet
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [Beignet] [PATCH i-g-t 2/2] configure: Bump required libdrm version to 2.4.60

[Jeff McGee]

On Tue, Mar 10, 2015 at 07:47:03PM +0100, Daniel Vetter wrote:
> On Tue, Mar 10, 2015 at 01:58:52PM -0400, Rob Clark wrote:
> > On Tue, Mar 10, 2015 at 12:59 PM, Jeff McGee  wrote:
> > > On Tue, Mar 10, 2015 at 08:37:30AM +0100, Daniel Vetter wrote:
> > >> On Mon, Mar 09, 2015 at 04:41:02PM -0700, jeff.mc...@intel.com wrote:
> > >> > From: Jeff McGee 
> > >> >
> > >> > tests/core_getparams needs the new libdrm interfaces for
> > >> > querying subslice and EU counts.
> > >> >
> > >> > For: VIZ-4636
> > >> > Signed-off-by: Jeff McGee 
> > >> > ---
> > >> >  configure.ac | 2 +-
> > >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >> >
> > >> > diff --git a/configure.ac b/configure.ac
> > >> > index 16d6a2e..88a1c3d 100644
> > >> > --- a/configure.ac
> > >> > +++ b/configure.ac
> > >> > @@ -82,7 +82,7 @@ if test "x$GCC" = "xyes"; then
> > >> >  fi
> > >> >  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
> > >> >
> > >> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.52 libdrm])
> > >> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.60 libdrm])
> > >>
> > >> Please don't and instead copypaste the new structs/defines with a local_
> > >> prefix like we do it for all the other new igt testcases. Forcing libdrm
> > >> to get updated for igt all the time can get annoying fast.
> > >> -Daniel
> > >>
> > > In this case I'm trying to exercise new API functions in libdrm which
> > > wrap the GETPARAM ioctl. Would you rather me bypass the wrapper to
> > > avoid requiring updated libdrm? I can do that, but it fails to test the
> > > complete path that client would use.
> > 
> > 
> > Am I missing something, or does 2.4.60 not exist yet?
> > 
> > That said dependency bumps for igt seem like less of an issue than
> > dependency bumps for mesa..  I mean if you are using igt you are
> > probably on the latest anyways..  I'm not sure why Daniel is so
> > concerned about that..
> > 
> > (but dependency bumps to something that doesn't exist yet should
> > perhaps be avoided)
> 
> I'd like to avoid massive depency loops for igt tests so that I can merge
> the testcase right when the patches land in -nightly. Otherwise there's
> always a small delay involved where regression can creep in. Also if I
> have to update libdrm every time I update igt that's annoying since
> without that I don't have to install/update anything at all - I run igt
> in-place. And we've used the LOCAL_ prefixes for pretty much every abi
> addition in igt tests thus far.
> -Daniel

I understand that and it certainly makes sense when libdrm is only
providing defines or structs. But as I said, in this case there is
code in libdrm (the wrapper) that we could test as part of the
complete path. Are you recommending that I implement duplicate
wrapper functions in igt with the local prefix?
-Jeff
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 2/2] tests/pm_sseu: Create new test pm_sseu

[jeff . mcgee]

From: Jeff McGee 

New test pm_sseu is intended for any subtest related to the
slice/subslice/EU power gating feature. The sole initial subtest,
'full-enable', confirms that the slice/subslice/EU state is at
full enablement when the render engine is active. Starting with
Gen9 SKL, the render power gating feature can leave SSEU in a
partially enabled state upon resumption of render work unless
explicit action is taken.

Signed-off-by: Jeff McGee 
---
 tests/.gitignore   |   1 +
 tests/Makefile.sources |   1 +
 tests/pm_sseu.c| 373 +
 3 files changed, 375 insertions(+)
 create mode 100644 tests/pm_sseu.c

diff --git a/tests/.gitignore b/tests/.gitignore
index 7b4dd94..23094ce 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -144,6 +144,7 @@ pm_psr
 pm_rc6_residency
 pm_rpm
 pm_rps
+pm_sseu
 prime_nv_api
 prime_nv_pcopy
 prime_nv_test
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 51e8376..74106c0 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -82,6 +82,7 @@ TESTS_progs_M = \
pm_rpm \
pm_rps \
pm_rc6_residency \
+   pm_sseu \
prime_self_import \
template \
$(NULL)
diff --git a/tests/pm_sseu.c b/tests/pm_sseu.c
new file mode 100644
index 000..45aeef3
--- /dev/null
+++ b/tests/pm_sseu.c
@@ -0,0 +1,373 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Jeff McGee 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "drmtest.h"
+#include "i915_drm.h"
+#include "intel_io.h"
+#include "intel_bufmgr.h"
+#include "intel_batchbuffer.h"
+#include "intel_chipset.h"
+#include "ioctl_wrappers.h"
+#include "igt_debugfs.h"
+#include "media_spin.h"
+
+static double
+to_dt(const struct timespec *start, const struct timespec *end)
+{
+   double dt;
+
+   dt = (end->tv_sec - start->tv_sec) * 1e3;
+   dt += (end->tv_nsec - start->tv_nsec) * 1e-6;
+
+   return dt;
+}
+
+struct status {
+   struct {
+   int slice_total;
+   int subslice_total;
+   int subslice_per;
+   int eu_total;
+   int eu_per;
+   bool has_slice_pg;
+   bool has_subslice_pg;
+   bool has_eu_pg;
+   } info;
+   struct {
+   int slice_total;
+   int subslice_total;
+   int subslice_per;
+   int eu_total;
+   int eu_per;
+   } hw;
+};
+
+#define DBG_STATUS_BUF_SIZE 4096
+
+struct {
+   int init;
+   int status_fd;
+   char status_buf[DBG_STATUS_BUF_SIZE];
+} dbg;
+
+static void
+dbg_get_status_section(const char *title, char **first, char **last)
+{
+   char *pos;
+
+   *first = strstr(dbg.status_buf, title);
+   igt_assert(*first != NULL);
+
+   pos = *first;
+   do {
+   pos = strchr(pos, '\n');
+   igt_assert(pos != NULL);
+   pos++;
+   } while (*pos == ' '); /* lines in the section begin with a space */
+   *last = pos - 1;
+}
+
+static int
+dbg_get_int(const char *first, const char *last, const char *name)
+{
+   char *pos;
+
+   pos = strstr(first, name);
+   igt_assert(pos != NULL);
+   pos = strstr(pos, ":");
+   igt_assert(pos != NULL);
+   pos += 2;
+   igt_assert(pos < last);
+
+   return strtol(pos, &pos, 10);
+}
+
+static bool
+dbg_get_bool(const char *first, const char *last, const char *name)
+{
+   char *pos;
+
+   pos = strstr(first, name);
+   igt_assert(pos != NULL);
+   pos = strstr(pos, ":");
+   igt_assert(pos != NULL);
+   pos += 2;
+   igt_assert(pos < last);
+
+   if (*pos == 'y')
+   return true;
+   if (*pos == 'n')
+   return false;
+
+   igt_assert(false);
+   return fa

[Intel-gfx] [PATCH i-g-t 1/2] lib: Add media spin

[jeff . mcgee]

From: Jeff McGee 

The media spin utility is derived from media fill. The purpose
is to create a simple means to keep the render engine (media
pipeline) busy for a controlled amount of time. It does so by
emitting a batch with a single execution thread that spins in
a tight loop the requested number of times. Each spin increments
a counter whose final 32-bit value is written to the destination
buffer on completion for checking. The implementation supports
Gen8, Gen8lp, and Gen9.

Signed-off-by: Jeff McGee 
---
 lib/Makefile.sources|   2 +
 lib/intel_batchbuffer.c |  24 +++
 lib/intel_batchbuffer.h |  22 ++
 lib/media_spin.c| 540 
 lib/media_spin.h|  39 
 5 files changed, 627 insertions(+)
 create mode 100644 lib/media_spin.c
 create mode 100644 lib/media_spin.h

diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 76f353a..3d93629 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -29,6 +29,8 @@ libintel_tools_la_SOURCES =   \
media_fill_gen8.c   \
media_fill_gen8lp.c \
media_fill_gen9.c   \
+   media_spin.h\
+   media_spin.c\
gen7_media.h\
gen8_media.h\
rendercopy_i915.c   \
diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
index c70f6d8..14970e4 100644
--- a/lib/intel_batchbuffer.c
+++ b/lib/intel_batchbuffer.c
@@ -39,6 +39,7 @@
 #include "intel_reg.h"
 #include "rendercopy.h"
 #include "media_fill.h"
+#include "media_spin.h"
 #include 
 
 /**
@@ -530,3 +531,26 @@ igt_fillfunc_t igt_get_gpgpu_fillfunc(int devid)
 
return fill;
 }
+
+/**
+ * igt_get_media_spinfunc:
+ * @devid: pci device id
+ *
+ * Returns:
+ *
+ * The platform-specific media spin function pointer for the device specified
+ * with @devid. Will return NULL when no media spin function is implemented.
+ */
+igt_media_spinfunc_t igt_get_media_spinfunc(int devid)
+{
+   igt_media_spinfunc_t spin = NULL;
+
+   if (IS_GEN9(devid))
+   spin = gen9_media_spinfunc;
+   else if (IS_BROADWELL(devid))
+   spin = gen8_media_spinfunc;
+   else if (IS_CHERRYVIEW(devid))
+   spin = gen8lp_media_spinfunc;
+
+   return spin;
+}
diff --git a/lib/intel_batchbuffer.h b/lib/intel_batchbuffer.h
index 12f7be1..13b356a 100644
--- a/lib/intel_batchbuffer.h
+++ b/lib/intel_batchbuffer.h
@@ -265,4 +265,26 @@ typedef void (*igt_fillfunc_t)(struct intel_batchbuffer 
*batch,
 igt_fillfunc_t igt_get_media_fillfunc(int devid);
 igt_fillfunc_t igt_get_gpgpu_fillfunc(int devid);
 
+/**
+ * igt_media_spinfunc_t:
+ * @batch: batchbuffer object
+ * @dst: destination i-g-t buffer object
+ * @spins: number of loops to execute
+ *
+ * This is the type of the per-platform media spin functions. The
+ * platform-specific implementation can be obtained by calling
+ * igt_get_media_spinfunc().
+ *
+ * The media spin function emits a batchbuffer for the render engine with
+ * the media pipeline selected. The workload consists of a single thread
+ * which spins in a tight loop the requested number of times. Each spin
+ * increments a counter whose final 32-bit value is written to the
+ * destination buffer on completion. This utility provides a simple way
+ * to keep the render engine busy for a set time for various tests.
+ */
+typedef void (*igt_media_spinfunc_t)(struct intel_batchbuffer *batch,
+struct igt_buf *dst, uint32_t spins);
+
+igt_media_spinfunc_t igt_get_media_spinfunc(int devid);
+
 #endif
diff --git a/lib/media_spin.c b/lib/media_spin.c
new file mode 100644
index 000..b44c55a
--- /dev/null
+++ b/lib/media_spin.c
@@ -0,0 +1,540 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ * Jeff McGee 
+ */
+
+#include 
+#include 
+#include 

Re: [Intel-gfx] [PATCH] drm/i915: Add polish to VLV WM shift+mask operations

[Jesse Barnes]

On 03/10/2015 07:16 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Wrap the FW register value shift+mask operations into a macro to hide
> the ugliness a bit. Also might avoid bugs due to typos.
> 
> Also rename all the primary/sprite plane low order bit masks to have the
> _VLV suffix, so that we can use the FW_WM_VLV() macro instead of the
> FW_WM() macro for them in a consistent manner. Cursor and all the high
> order bits are left to use the FW_WM() macro as there's no real
> confusion with them.
> 
> Cc: Daniel Vetter 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 10 +++---
>  drivers/gpu/drm/i915/intel_pm.c | 74 
> +++--
>  2 files changed, 46 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 495b22b..8ff039d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4161,25 +4161,25 @@ enum skl_disp_power_wells {
>  #define   DSPFW_SPRITED_WM1_SHIFT24
>  #define   DSPFW_SPRITED_WM1_MASK (0xff<<24)
>  #define   DSPFW_SPRITED_SHIFT16
> -#define   DSPFW_SPRITED_MASK (0xff<<16)
> +#define   DSPFW_SPRITED_MASK_VLV (0xff<<16)
>  #define   DSPFW_SPRITEC_WM1_SHIFT8
>  #define   DSPFW_SPRITEC_WM1_MASK (0xff<<8)
>  #define   DSPFW_SPRITEC_SHIFT0
> -#define   DSPFW_SPRITEC_MASK (0xff<<0)
> +#define   DSPFW_SPRITEC_MASK_VLV (0xff<<0)
>  #define DSPFW8_CHV   (VLV_DISPLAY_BASE + 0x700b8)
>  #define   DSPFW_SPRITEF_WM1_SHIFT24
>  #define   DSPFW_SPRITEF_WM1_MASK (0xff<<24)
>  #define   DSPFW_SPRITEF_SHIFT16
> -#define   DSPFW_SPRITEF_MASK (0xff<<16)
> +#define   DSPFW_SPRITEF_MASK_VLV (0xff<<16)
>  #define   DSPFW_SPRITEE_WM1_SHIFT8
>  #define   DSPFW_SPRITEE_WM1_MASK (0xff<<8)
>  #define   DSPFW_SPRITEE_SHIFT0
> -#define   DSPFW_SPRITEE_MASK (0xff<<0)
> +#define   DSPFW_SPRITEE_MASK_VLV (0xff<<0)
>  #define DSPFW9_CHV   (VLV_DISPLAY_BASE + 0x7007c) /* wtf #2? */
>  #define   DSPFW_PLANEC_WM1_SHIFT 24
>  #define   DSPFW_PLANEC_WM1_MASK  (0xff<<24)
>  #define   DSPFW_PLANEC_SHIFT 16
> -#define   DSPFW_PLANEC_MASK  (0xff<<16)
> +#define   DSPFW_PLANEC_MASK_VLV  (0xff<<16)
>  #define   DSPFW_CURSORC_WM1_SHIFT8
>  #define   DSPFW_CURSORC_WM1_MASK (0x3f<<16)
>  #define   DSPFW_CURSORC_SHIFT0
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0e84558..8ac358d0 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -835,6 +835,11 @@ static bool g4x_compute_srwm(struct drm_device *dev,
> display, cursor);
>  }
>  
> +#define FW_WM(value, plane) \
> + (((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK)
> +#define FW_WM_VLV(value, plane) \
> + (((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK_VLV)
> +
>  static void vlv_write_wm_values(struct intel_crtc *crtc,
>   const struct vlv_wm_values *wm)
>  {
> @@ -848,50 +853,50 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
>  (wm->ddl[pipe].primary << DDL_PLANE_SHIFT));
>  
>   I915_WRITE(DSPFW1,
> -((wm->sr.plane << DSPFW_SR_SHIFT) & DSPFW_SR_MASK) |
> -((wm->pipe[PIPE_B].cursor << DSPFW_CURSORB_SHIFT) & 
> DSPFW_CURSORB_MASK) |
> -((wm->pipe[PIPE_B].primary << DSPFW_PLANEB_SHIFT) & 
> DSPFW_PLANEB_MASK_VLV) |
> -((wm->pipe[PIPE_A].primary << DSPFW_PLANEA_SHIFT) & 
> DSPFW_PLANEA_MASK_VLV));
> +FW_WM(wm->sr.plane, SR) |
> +FW_WM(wm->pipe[PIPE_B].cursor, CURSORB) |
> +FW_WM_VLV(wm->pipe[PIPE_B].primary, PLANEB) |
> +FW_WM_VLV(wm->pipe[PIPE_A].primary, PLANEA));
>   I915_WRITE(DSPFW2,
> -((wm->pipe[PIPE_A].sprite[1] << DSPFW_SPRITEB_SHIFT) & 
> DSPFW_SPRITEB_MASK_VLV) |
> -((wm->pipe[PIPE_A].cursor << DSPFW_CURSORA_SHIFT) & 
> DSPFW_CURSORA_MASK) |
> -((wm->pipe[PIPE_A].sprite[0] << DSPFW_SPRITEA_SHIFT) & 
> DSPFW_SPRITEA_MASK_VLV));
> +FW_WM_VLV(wm->pipe[PIPE_A].sprite[1], SPRITEB) |
> +FW_WM(wm->pipe[PIPE_A].cursor, CURSORA) |
> +FW_WM_VLV(wm->pipe[PIPE_A].sprite[0], SPRITEA));
>   I915_WRITE(DSPFW3,
> -((wm->sr.cursor << DSPFW_CURSOR_SR_SHIFT) & 
> DSPFW_CURSOR_SR_MASK));
> +FW_WM(wm->sr.cursor, CURSOR_SR));
>  
>   if (IS_CHERRYVIEW(dev_priv)) {
>   I915_WRITE(DSPFW7_CHV,
> -((wm->pipe[PIPE_B].sprite[1] << DSPFW_SPRITED_SHIFT) 
> & DSPFW_SPRITED_MASK) |
> -((wm->pipe[PIPE_B].sprite[0] << DSPFW_SPRITEC_SHIFT) 
> & DSPFW_SPRITEC_MASK));
> +  

Re: [Intel-gfx] [PATCH 7/9] drm/i915: Update watermarks after the derived plane state is uptodate

[Daniel Vetter]

On Tue, Mar 10, 2015 at 10:13:52AM -0700, Matt Roper wrote:
> On Tue, Mar 10, 2015 at 01:15:27PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> > 
> > When enabling planes during .crtc_enable() we currently want to update
> > the watermarks before enabling the planes. We already do it once just
> > before enabling the pipe, but at that point out derived plane state is
> > still out of whack, so we need to do it again after the .atomic_check()
> > hooks have been called.
> > 
> > What this means is now we could actually start to trust the derived
> > plane state (clipped size, 'visible', etc.) in the watermark code.
> > 
> > The pre pipe enable watermark update is supposed to be just make sure
> > the other pipes are ready to have their FIFOs potentially reduced, so we
> > need to keep it there as well.
> > 
> > Since we don't yet have proper two-part watermark update leave the
> > watermakrs alone in the plane disable case. This way they'll get updated
> > only after the planes and pipe have all been turned off.
> > 
> > Cc: Matt Roper 
> > Signed-off-by: Ville Syrjälä 
> 
> Reviewed-by: Matt Roper 

Since I'm not sold on the atomic_check changes in your series I'll hold
off on this one for now.
-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/9] drm/i915: Make derived plane state correct after crtc_enable

[Daniel Vetter]

On Wed, Mar 11, 2015 at 10:52:29AM +0100, Daniel Vetter wrote:
> On Tue, Mar 10, 2015 at 07:57:13PM +0200, Ville Syrjälä wrote:
> > On Tue, Mar 10, 2015 at 10:01:51AM -0700, Matt Roper wrote:
> > > On Tue, Mar 10, 2015 at 01:15:24PM +0200, ville.syrj...@linux.intel.com 
> > > wrote:
> > > > From: Ville Syrjälä 
> > > > -static void disable_plane_internal(struct drm_plane *plane)
> > > > +static void _intel_crtc_enable_planes(struct intel_crtc *crtc)
> > > >  {
> > > > -   struct intel_plane *intel_plane = to_intel_plane(plane);
> > > > -   struct drm_plane_state *state =
> > > > -   plane->funcs->atomic_duplicate_state(plane);
> > > > -   struct intel_plane_state *intel_state = 
> > > > to_intel_plane_state(state);
> > > > +   struct drm_device *dev = crtc->base.dev;
> > > > +   enum pipe pipe = crtc->pipe;
> > > > +   struct intel_plane *plane;
> > > > +   const struct drm_crtc_helper_funcs *crtc_funcs =
> > > > +   crtc->base.helper_private;
> > > >  
> > > > -   intel_state->visible = false;
> > > > -   intel_plane->commit_plane(plane, intel_state);
> > > > +   for_each_intel_plane(dev, plane) {
> > > > +   const struct drm_plane_helper_funcs *funcs =
> > > > +   plane->base.helper_private;
> > > > +   struct intel_plane_state *state =
> > > > +   to_intel_plane_state(plane->base.state);
> > > >  
> > > > -   intel_plane_destroy_state(plane, state);
> > > > +   if (plane->pipe != pipe)
> > > > +   continue;
> > > > +
> > > > +   if (funcs->atomic_check(&plane->base, &state->base))
> > > 
> > > Maybe add a WARN_ON() here?  I'm assuming that this shouldn't really be
> > > possible since if this fails it means we've already previously done a
> > > commit of invalid state on a previous atomic transaction.  But if it
> > > does somehow happen, the WARN will give us a clue why the plane contents
> > > simply didn't show up.
> > 
> > I can think of one way to make it fail. That is, first set a smaller
> > mode with the primary plane (and fb) configured to cover that fully, and
> > then switch to a larger mode without reconfiguring the primary plane. If
> > the hardware requires the primary plane to be fullscreen it'll fail. But
> > that should actaully not be possible using the legacy modeset API as it
> > always reconfigures the primary, so we'd only have to worry about that
> > with full atomic modeset, and for that we anyway need to change the code
> > to do the check stuff up front.
> > 
> > So yeah, with the way things are this should not be able to fail. I'll
> > respin with the WARN.
> 
> I haven't fully dug into the details here, but a few randome comments:
> - While transitioning we're calling the transitional plane helpers, which
>   should call the atomic_check stuff for us on the primary plane. If we
>   need to call atomic_check on other planes too (why?) then I think that
>   should be done as close as possible to where we do that for the primary
>   one. Since eventually we need to unbury that call again.
> 
> - I don't like frobbing state objects which are committed (i.e. updating
>   visible like here), they're supposed to be invariant. With proper atomic
>   the way to deal with that is to grab all the required plane states and
>   put them into the drm_atomic_state update structure.
> 
> - My idea for resolving our current nesting issues with
>   enable/disable_planes functions was two parts: a) open-code a hardcoded
>   disable-all-planes function by just calling plane disable code
>   unconditionally. Iirc there's been patches once somewhere to do that
>   split in i915 (maybe I'm dreaming), but this use-case is also why I
>   added the atomic_plane_disable hook. b) on restore we just do a normal
>   plane commit with the unchanged plane states, they should all still
>   work.
> 
> btw if we wire up the atomic_disable_plane hook then we can rip out
> intel_plane_atomic_update. The "don't disable twice" check is already done
> by the helpers in that case.
> 
> I'll grab some coffee and see what's all wrong with my ideas here now, but
> please bring in critique too ;-)

One immediate problem is that we key off from intel_state->visible to
decide whether to enable or not, and the core helpers key off from
state->fb. So I think we'd need to roll our own, but with the same idea
of splitting out an explicit plane_disable hook.

Or maybe we should add a drm_plane_state->visible derived state which
helpers fill out to match drm_plane_state->fb by default. That might be
even neater, and matches somewhat how we allow drivers to overwrite
crtc_state->needs_modeset to control which hooks the helpers will call.

Clearly, more coffee is neede here ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Inte

Re: [Intel-gfx] [PATCH 4/9] drm/i915: Make derived plane state correct after crtc_enable

[Ville Syrjälä]

On Wed, Mar 11, 2015 at 10:52:29AM +0100, Daniel Vetter wrote:
> On Tue, Mar 10, 2015 at 07:57:13PM +0200, Ville Syrjälä wrote:
> > On Tue, Mar 10, 2015 at 10:01:51AM -0700, Matt Roper wrote:
> > > On Tue, Mar 10, 2015 at 01:15:24PM +0200, ville.syrj...@linux.intel.com 
> > > wrote:
> > > > From: Ville Syrjälä 
> > > > -static void disable_plane_internal(struct drm_plane *plane)
> > > > +static void _intel_crtc_enable_planes(struct intel_crtc *crtc)
> > > >  {
> > > > -   struct intel_plane *intel_plane = to_intel_plane(plane);
> > > > -   struct drm_plane_state *state =
> > > > -   plane->funcs->atomic_duplicate_state(plane);
> > > > -   struct intel_plane_state *intel_state = 
> > > > to_intel_plane_state(state);
> > > > +   struct drm_device *dev = crtc->base.dev;
> > > > +   enum pipe pipe = crtc->pipe;
> > > > +   struct intel_plane *plane;
> > > > +   const struct drm_crtc_helper_funcs *crtc_funcs =
> > > > +   crtc->base.helper_private;
> > > >  
> > > > -   intel_state->visible = false;
> > > > -   intel_plane->commit_plane(plane, intel_state);
> > > > +   for_each_intel_plane(dev, plane) {
> > > > +   const struct drm_plane_helper_funcs *funcs =
> > > > +   plane->base.helper_private;
> > > > +   struct intel_plane_state *state =
> > > > +   to_intel_plane_state(plane->base.state);
> > > >  
> > > > -   intel_plane_destroy_state(plane, state);
> > > > +   if (plane->pipe != pipe)
> > > > +   continue;
> > > > +
> > > > +   if (funcs->atomic_check(&plane->base, &state->base))
> > > 
> > > Maybe add a WARN_ON() here?  I'm assuming that this shouldn't really be
> > > possible since if this fails it means we've already previously done a
> > > commit of invalid state on a previous atomic transaction.  But if it
> > > does somehow happen, the WARN will give us a clue why the plane contents
> > > simply didn't show up.
> > 
> > I can think of one way to make it fail. That is, first set a smaller
> > mode with the primary plane (and fb) configured to cover that fully, and
> > then switch to a larger mode without reconfiguring the primary plane. If
> > the hardware requires the primary plane to be fullscreen it'll fail. But
> > that should actaully not be possible using the legacy modeset API as it
> > always reconfigures the primary, so we'd only have to worry about that
> > with full atomic modeset, and for that we anyway need to change the code
> > to do the check stuff up front.
> > 
> > So yeah, with the way things are this should not be able to fail. I'll
> > respin with the WARN.
> 
> I haven't fully dug into the details here, but a few randome comments:
> - While transitioning we're calling the transitional plane helpers, which
>   should call the atomic_check stuff for us on the primary plane. If we
>   need to call atomic_check on other planes too (why?)

Because we want the derived state to be updated to match the (potentially
changed) crtc config. We do call the .update_plane() hook from the
modeset path, but that happens at a time when the pipe is off, so our
clipping calculations end up saying the plane is invisible. I think fixing
that the right way pretty much involves the atomic conversion of the
modeset path.

>   then I think that
>   should be done as close as possible to where we do that for the primary
>   one. Since eventually we need to unbury that call again.

With my patch _all_ planes get their .atomic_check() called in the same
place (during plane enable phase of .crtc_enable()).

> 
> - I don't like frobbing state objects which are committed (i.e. updating
>   visible like here), they're supposed to be invariant. With proper atomic
>   the way to deal with that is to grab all the required plane states and
>   put them into the drm_atomic_state update structure.

We really want to frob it so that the derived state will reflect
reality. Most importantly state->visible should be false whenever the
pipe is off, otherwise we can't trust state->visible and would also need
go look at the crtc state whenever we're trying to decide if the plane
is actually on or not.

As for the direct state frobbing, we could make a copy I guess and frob
that instead, and then commit it. But that seems a lot of work for no gain.

> 
> - My idea for resolving our current nesting issues with
>   enable/disable_planes functions was two parts: a) open-code a hardcoded
>   disable-all-planes function by just calling plane disable code
>   unconditionally. Iirc there's been patches once somewhere to do that
>   split in i915 (maybe I'm dreaming), but this use-case is also why I
>   added the atomic_plane_disable hook. b) on restore we just do a normal
>   plane commit with the unchanged plane states, they should all still
>   work.
> 
> btw if we wire up the atomic_disable_plane hook then we can rip out
> intel_plane_atomic_update. The

Re: [Intel-gfx] [PATCH v2] drm/i915: Fix BUG in i915_gem.c when switch to console

[Jani Nikula]

On Thu, 12 Mar 2015, Xi Ruoyao  wrote:
> In intel_crtc_page_flip, intel_display.c, the code changed the framebuffer
> assigned to plane crtc->primary by
>
> crtc->primary->fb = fb;
>
> However, it forgot to change crtc->primary->state->fb. However, when we
> switch to console, some kernel code will read crtc->primary->state->fb
> to get the framebuffer assigned to crtc->primaty. Then a framebuffer
> object can be unpinned twice and a kernel BUG will be produced in i915_gem.c.
>
> So, update crtc->primary->state->fb in intel_display.c using
> drm_atomic_set_fb_for_plane to fix the BUG.
>
> Signed-off-by: Xi Ruoyao 
> Fixed: Bug 93711 

Is this a problem with drm-intel-nightly? In particular see

commit afd65eb4cc0578a9c07d621acdb8a570e2782bf7
Author: Matt Roper 
Date:   Tue Feb 3 13:10:04 2015 -0800

drm/i915: Ensure plane->state->fb stays in sync with plane->fb

Matt, do you think this fixes the described issue? Can we backport to
drm-intel-fixes (and v4.0)?

BR,
Jani.



> ---
>  Sorry, the previous patch is mangled by email client, so I am re-sending it.
>
>  drivers/gpu/drm/i915/intel_display.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index e730789..97083fd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -37,6 +37,7 @@
>  #include 
>  #include "i915_drv.h"
>  #include "i915_trace.h"
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -9816,6 +9817,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>   drm_gem_object_reference(&obj->base);
>  
>   crtc->primary->fb = fb;
> + drm_atomic_set_fb_for_plane(crtc->primary->state, fb);
>  
>   work->pending_flip_obj = obj;
>  
> -- 
> 1.9.1
>

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

[Intel-gfx] [PULL] drm-intel-fixes

[Jani Nikula]

Hi Dave -

More i915 fixes, three out of four are fixes to old bugs, cc: stable.

BR,
Jani.

The following changes since commit 9eccca0843205f87c00404b663188b88eb248051:

  Linux 4.0-rc3 (2015-03-08 16:09:09 -0700)

are available in the git repository at:

  git://anongit.freedesktop.org/drm-intel tags/drm-intel-fixes-2015-03-12

for you to fetch changes up to 5e4f518959bdf8a4f9c8f80879e4a0f7a95d2cb3:

  drm/i915: Prevent TLB error on first execution on SNB (2015-03-10 15:30:23 
+0200)


Chris Wilson (2):
  drm/i915: Make WAIT_IOCTL negative timeouts be indefinite again
  drm/i915: Prevent TLB error on first execution on SNB

Dave Gordon (1):
  drm/i915: use in_interrupt() not in_irq() to check context

Mika Kuoppala (1):
  drm/i915: Do both mt and gen6 style forcewake reset on ivb probe

 drivers/gpu/drm/i915/i915_gem.c  | 25 -
 drivers/gpu/drm/i915/intel_display.c |  2 +-
 drivers/gpu/drm/i915/intel_uncore.c  |  8 +++-
 3 files changed, 28 insertions(+), 7 deletions(-)

-- 
Jani Nikula, 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 4/9] drm/i915: Make derived plane state correct after crtc_enable

[Ville Syrjälä]

On Wed, Mar 11, 2015 at 05:23:27PM +0100, Daniel Vetter wrote:
> On Wed, Mar 11, 2015 at 02:19:38PM +0200, Ville Syrjälä wrote:
> > On Wed, Mar 11, 2015 at 11:24:34AM +0100, Daniel Vetter wrote:
> > > On Wed, Mar 11, 2015 at 12:05:39PM +0200, Ville Syrjälä wrote:
> > > > On Wed, Mar 11, 2015 at 10:52:29AM +0100, Daniel Vetter wrote:
> > > > > On Tue, Mar 10, 2015 at 07:57:13PM +0200, Ville Syrjälä wrote:
> > > > > > On Tue, Mar 10, 2015 at 10:01:51AM -0700, Matt Roper wrote:
> > > > > > > On Tue, Mar 10, 2015 at 01:15:24PM +0200, 
> > > > > > > ville.syrj...@linux.intel.com wrote:
> > > > > > > > From: Ville Syrjälä 
> > > > > > > > -static void disable_plane_internal(struct drm_plane *plane)
> > > > > > > > +static void _intel_crtc_enable_planes(struct intel_crtc *crtc)
> > > > > > > >  {
> > > > > > > > -   struct intel_plane *intel_plane = to_intel_plane(plane);
> > > > > > > > -   struct drm_plane_state *state =
> > > > > > > > -   plane->funcs->atomic_duplicate_state(plane);
> > > > > > > > -   struct intel_plane_state *intel_state = 
> > > > > > > > to_intel_plane_state(state);
> > > > > > > > +   struct drm_device *dev = crtc->base.dev;
> > > > > > > > +   enum pipe pipe = crtc->pipe;
> > > > > > > > +   struct intel_plane *plane;
> > > > > > > > +   const struct drm_crtc_helper_funcs *crtc_funcs =
> > > > > > > > +   crtc->base.helper_private;
> > > > > > > >  
> > > > > > > > -   intel_state->visible = false;
> > > > > > > > -   intel_plane->commit_plane(plane, intel_state);
> > > > > > > > +   for_each_intel_plane(dev, plane) {
> > > > > > > > +   const struct drm_plane_helper_funcs *funcs =
> > > > > > > > +   plane->base.helper_private;
> > > > > > > > +   struct intel_plane_state *state =
> > > > > > > > +   to_intel_plane_state(plane->base.state);
> > > > > > > >  
> > > > > > > > -   intel_plane_destroy_state(plane, state);
> > > > > > > > +   if (plane->pipe != pipe)
> > > > > > > > +   continue;
> > > > > > > > +
> > > > > > > > +   if (funcs->atomic_check(&plane->base, 
> > > > > > > > &state->base))
> > > > > > > 
> > > > > > > Maybe add a WARN_ON() here?  I'm assuming that this shouldn't 
> > > > > > > really be
> > > > > > > possible since if this fails it means we've already previously 
> > > > > > > done a
> > > > > > > commit of invalid state on a previous atomic transaction.  But if 
> > > > > > > it
> > > > > > > does somehow happen, the WARN will give us a clue why the plane 
> > > > > > > contents
> > > > > > > simply didn't show up.
> > > > > > 
> > > > > > I can think of one way to make it fail. That is, first set a smaller
> > > > > > mode with the primary plane (and fb) configured to cover that 
> > > > > > fully, and
> > > > > > then switch to a larger mode without reconfiguring the primary 
> > > > > > plane. If
> > > > > > the hardware requires the primary plane to be fullscreen it'll 
> > > > > > fail. But
> > > > > > that should actaully not be possible using the legacy modeset API 
> > > > > > as it
> > > > > > always reconfigures the primary, so we'd only have to worry about 
> > > > > > that
> > > > > > with full atomic modeset, and for that we anyway need to change the 
> > > > > > code
> > > > > > to do the check stuff up front.
> > > > > > 
> > > > > > So yeah, with the way things are this should not be able to fail. 
> > > > > > I'll
> > > > > > respin with the WARN.
> > > > > 
> > > > > I haven't fully dug into the details here, but a few randome comments:
> > > > > - While transitioning we're calling the transitional plane helpers, 
> > > > > which
> > > > >   should call the atomic_check stuff for us on the primary plane. If 
> > > > > we
> > > > >   need to call atomic_check on other planes too (why?)
> > > > 
> > > > Because we want the derived state to be updated to match the 
> > > > (potentially
> > > > changed) crtc config. We do call the .update_plane() hook from the
> > > > modeset path, but that happens at a time when the pipe is off, so our
> > > > clipping calculations end up saying the plane is invisible. I think 
> > > > fixing
> > > > that the right way pretty much involves the atomic conversion of the
> > > > modeset path.
> > > 
> > > Why do we conclude it's invisible?
> > 
> > Because crtc->active. So for this we'll be wanting crtc_state->active
> > or somesuch which tells us upfront whether the pipe is going to be
> > active or not.
> > 
> > But that's also beside the point a bit since we still want to make call
> > the .atomic_check() for all planes. Right now we'd call it for primary
> > (at the wrong point wrt. crtc->active) and we call it for sprites later
> > when crtc->active is showing the right state, but we don't call it at
> > all for cursors. That's why we still have that ad-hoc extra cursor
> > clipping code in intel_update_cursor(). If we could make the der

Re: [Intel-gfx] [PATCH v2] drm/i915: Optimistically spin for the request completion

[Chris Wilson]

On Wed, Mar 11, 2015 at 09:18:19PM +, Chris Wilson wrote:
> Arguably busy-spinning on an idle system isn't totally evil, but it
> certainly is likely to come at a power cost. On the other hand, spinning
> is relatively rare outside of benchmarks. Rare enough to be useful?

As a counterpoint, I plugged in the qm_pos trick we use for lowlatency
dma interrupts (gmbus, dp-aux) and found no improvement at all over a
plain interrupt driven __i915_wait_request.
-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 11/11] drm/i915/skl: Enable the RPS interrupts programming

[Daniel Vetter]

On Wed, Mar 11, 2015 at 02:12:02PM -0700, Jesse Barnes wrote:
> On 03/05/2015 09:37 PM, akash.g...@intel.com wrote:
> > From: Akash Goel 
> > 
> > Enable the RPS interrupts programming(enable/disable/reset) for GEN9,
> > as missing changes to enable the RPS support on GEN9 have been added.
> > 
> > Signed-off-by: Akash Goel 
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 17 +++--
> >  1 file changed, 3 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c 
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 6273c282..3692837 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5629,12 +5629,7 @@ static void gen6_suspend_rps(struct drm_device *dev)
> >  
> > flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> >  
> > -   /*
> > -* TODO: disable RPS interrupts on GEN9+ too once RPS support
> > -* is added for it.
> > -*/
> > -   if (INTEL_INFO(dev)->gen < 9)
> > -   gen6_disable_rps_interrupts(dev);
> > +   gen6_disable_rps_interrupts(dev);
> >  }
> >  
> >  /**
> > @@ -5692,12 +5687,7 @@ static void intel_gen6_powersave_work(struct 
> > work_struct *work)
> >  
> > mutex_lock(&dev_priv->rps.hw_lock);
> >  
> > -   /*
> > -* TODO: reset/enable RPS interrupts on GEN9+ too, once RPS support is
> > -* added for it.
> > -*/
> > -   if (INTEL_INFO(dev)->gen < 9)
> > -   gen6_reset_rps_interrupts(dev);
> > +   gen6_reset_rps_interrupts(dev);
> >  
> > if (IS_CHERRYVIEW(dev)) {
> > cherryview_enable_rps(dev);
> > @@ -5716,8 +5706,7 @@ static void intel_gen6_powersave_work(struct 
> > work_struct *work)
> > }
> > dev_priv->rps.enabled = true;
> >  
> > -   if (INTEL_INFO(dev)->gen < 9)
> > -   gen6_enable_rps_interrupts(dev);
> > +   gen6_enable_rps_interrupts(dev);
> >  
> > mutex_unlock(&dev_priv->rps.hw_lock);
> >  
> > 
> 
> Reviewed-by: Jesse Barnes 

Merged the remaining four, thanks for patches&review.
-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: Optimistically spin for the request completion

[Daniel Vetter]

On Wed, Mar 11, 2015 at 09:18:19PM +, Chris Wilson wrote:
> On Wed, Mar 11, 2015 at 03:29:19PM +, Chris Wilson wrote:
> > +   while (!need_resched()) {
> > +   if (i915_gem_request_completed(req, true)) {
> > +   ret = 0;
> > +   goto out;
> > +   }
> > +
> > +   if (timeout && time_after_eq(jiffies, timeout))
> > +   break;
> > +
> > +   cpu_relax_lowlatency();
> > +   }
> 
> Hmm. This unfortunately doesn't quite work the same as the optimistic
> mutex spinner. The difference being that the scheduler knows that two
> processes are contending for the mutex, but it doesn't know about the
> contention between the thread and the GPU. The result is that unless
> there is other activity on the system we simply degrade into a busy-spin
> until the GPU is finished, rather than what I intended as spin for the
> next ~10ms and then fallback to the interrupt.
> 
> Arguably busy-spinning on an idle system isn't totally evil, but it
> certainly is likely to come at a power cost. On the other hand, spinning
> is relatively rare outside of benchmarks. Rare enough to be useful?

Maybe throw in a limit of a few useconds under the assumption that it's ok
to spin for roughly the average sleep+wakeup latency? And once spun out go
to sleep?
-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 2/2] drm/i915: Rewrite IVB FDI bifurcation conflict checks

[Daniel Vetter]

On Thu, Mar 12, 2015 at 09:35:33AM +0200, Ander Conselvan De Oliveira wrote:
> (for the series)
> Reviewed-by: Ander Conselvan de Oliveira 

Both merged to dinq, thanks.
-Daniel

> 
> On Wed, 2015-03-11 at 18:52 +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> > 
> > Ignore the current state of the pipe and just check crtc_state->enable
> > and the number of FDI lanes required. This means we don't accidentally
> > mistake the FDI lanes as being available of one of the pipes just
> > happens to be disabled at the time of the check. Also we no longer
> > consider pipe C to require FDI lanes when it's driving the eDP
> > transcoder.
> > 
> > We also take the opportunity to make the code a bit nicer looking by
> > hiding the ugly bits in the new pipe_required_fdi_lanes() function.
> > 
> > Cc: Ander Conselvan de Oliveira 
> > Cc: Daniel Vetter 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 29 +++--
> >  1 file changed, 15 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 242a8a7..72e9816 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3152,12 +3152,6 @@ static void intel_fdi_normal_train(struct drm_crtc 
> > *crtc)
> >FDI_FE_ERRC_ENABLE);
> >  }
> >  
> > -static bool pipe_has_enabled_pch(struct intel_crtc *crtc)
> > -{
> > -   return crtc->base.state->enable && crtc->active &&
> > -   crtc->config->has_pch_encoder;
> > -}
> > -
> >  /* The FDI link training functions for ILK/Ibexpeak. */
> >  static void ironlake_fdi_link_train(struct drm_crtc *crtc)
> >  {
> > @@ -5548,13 +5542,21 @@ bool intel_connector_get_hw_state(struct 
> > intel_connector *connector)
> > return encoder->get_hw_state(encoder, &pipe);
> >  }
> >  
> > +static int pipe_required_fdi_lanes(struct drm_device *dev, enum pipe pipe)
> > +{
> > +   struct intel_crtc *crtc =
> > +   to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> > +
> > +   if (crtc->base.state->enable &&
> > +   crtc->config->has_pch_encoder)
> > +   return crtc->config->fdi_lanes;
> > +
> > +   return 0;
> > +}
> > +
> >  static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe 
> > pipe,
> >  struct intel_crtc_state *pipe_config)
> >  {
> > -   struct drm_i915_private *dev_priv = dev->dev_private;
> > -   struct intel_crtc *pipe_B_crtc =
> > -   to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
> > -
> > DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
> >   pipe_name(pipe), pipe_config->fdi_lanes);
> > if (pipe_config->fdi_lanes > 4) {
> > @@ -5581,8 +5583,8 @@ static bool ironlake_check_fdi_lanes(struct 
> > drm_device *dev, enum pipe pipe,
> > case PIPE_A:
> > return true;
> > case PIPE_B:
> > -   if (dev_priv->pipe_to_crtc_mapping[PIPE_C]->enabled &&
> > -   pipe_config->fdi_lanes > 2) {
> > +   if (pipe_config->fdi_lanes > 2 &&
> > +   pipe_required_fdi_lanes(dev, PIPE_C) > 0) {
> > DRM_DEBUG_KMS("invalid shared fdi lane config on pipe 
> > %c: %i lanes\n",
> >   pipe_name(pipe), pipe_config->fdi_lanes);
> > return false;
> > @@ -5594,8 +5596,7 @@ static bool ironlake_check_fdi_lanes(struct 
> > drm_device *dev, enum pipe pipe,
> >   pipe_name(pipe), pipe_config->fdi_lanes);
> > return false;
> > }
> > -   if (pipe_has_enabled_pch(pipe_B_crtc) &&
> > -   pipe_B_crtc->config->fdi_lanes > 2) {
> > +   if (pipe_required_fdi_lanes(dev, PIPE_B) > 2) {
> > DRM_DEBUG_KMS("fdi link B uses too many lanes to enable 
> > link C\n");
> > return false;
> > }
> 
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH] drm/i915: Read CHV_PLL_DW8 from the correct offset

[Daniel Vetter]

On Wed, Mar 11, 2015 at 08:20:44PM -0700, Todd Previte wrote:
> On 3/11/2015 1:52 PM, ville.syrj...@linux.intel.com wrote:
> >From: Ville Syrjälä 
> >
> >We accidentally pass 'pipe' instead of 'port' to CHV_PLL_DW8() and
> >with PIPE_C we end up at register offset 0x8320 which isn't the
> >0x8020 we wanted. Fix it.
> >
> >The problem was fortunately caught by the sanity check in vlv_dpio_read():
> >WARNING: CPU: 1 PID: 238 at ../drivers/gpu/drm/i915/intel_sideband.c:200 
> >vlv_dpio_read+0x77/0x80 [i915]()
> >DPIO read pipe C reg 0x8320 == 0x
> >
> >The problem got introduced with this commit:
> >  commit 71af07f91f12bbab96335e202c82525d31680960
> >  Author: Vijay Purushothaman 
> >  Date:   Thu Mar 5 19:33:08 2015 +0530
> >
> > drm/i915: Update prop, int co-eff and gain threshold for CHV
> >
> >Cc: Vijay Purushothaman 
> >Signed-off-by: Ville Syrjälä 
> >---
> >  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 ecbad5a..198e5fc 100644
> >--- a/drivers/gpu/drm/i915/intel_display.c
> >+++ b/drivers/gpu/drm/i915/intel_display.c
> >@@ -6270,7 +6270,7 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
> > }
> > vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW6(port), loopfilter);
> >-dpio_val = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW8(pipe));
> >+dpio_val = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW8(port));
> > dpio_val &= ~DPIO_CHV_TDC_TARGET_CNT_MASK;
> > dpio_val |= (tribuf_calcntr << DPIO_CHV_TDC_TARGET_CNT_SHIFT);
> > vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW8(port), dpio_val);
> 
> Looks good to me.
> 
> Reviewed-by: Todd Previte 

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

Re: [Intel-gfx] [PATCH 07/11] drm/i915/skl: Updated the gen9_enable_rps function

[Chris Wilson]

On Wed, Mar 11, 2015 at 12:27:59PM -0700, Jesse Barnes wrote:
> On 03/05/2015 09:37 PM, akash.g...@intel.com wrote:
> > +   /* Leaning on the below call to gen6_set_rps to program/setup the
> > +* Up/Down EI & threshold registers, as well as the RP_CONTROL,
> > +* RP_INTERRUPT_LIMITS & RPNSWREQ registers */
> > +   dev_priv->rps.power = HIGH_POWER; /* force a reset */
> 
> Are you also sure that dev_priv->rps.cur_freq != min_freq_softlimit at
> this point?  That's the condition for calling into the threshold update
> function (maybe gen6_set_rps should check both variables though).

It's a good point, but Akash has inherited that bug from me. What I
think we want is removing the actual intel_set_rps() calls here (and the
rest of the *_enable_rps()) and do an intel_set_rps_idle() call from the
common point in the caller, where we can put all the dancing required to
force the RPS initialisation.
-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 07/11] drm/i915/skl: Updated the gen9_enable_rps function

[Jesse Barnes]

On 03/11/2015 12:46 PM, Chris Wilson wrote:
> On Wed, Mar 11, 2015 at 12:27:59PM -0700, Jesse Barnes wrote:
>> On 03/05/2015 09:37 PM, akash.g...@intel.com wrote:
>>> +   /* Leaning on the below call to gen6_set_rps to program/setup the
>>> +* Up/Down EI & threshold registers, as well as the RP_CONTROL,
>>> +* RP_INTERRUPT_LIMITS & RPNSWREQ registers */
>>> +   dev_priv->rps.power = HIGH_POWER; /* force a reset */
>>
>> Are you also sure that dev_priv->rps.cur_freq != min_freq_softlimit at
>> this point?  That's the condition for calling into the threshold update
>> function (maybe gen6_set_rps should check both variables though).
> 
> It's a good point, but Akash has inherited that bug from me. What I
> think we want is removing the actual intel_set_rps() calls here (and the
> rest of the *_enable_rps()) and do an intel_set_rps_idle() call from the
> common point in the caller, where we can put all the dancing required to
> force the RPS initialisation.

Yeah, that would make things a little clearer.  I'd be fine with that as
a patch on top, fixing up the other functions as well.

Jesse

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

Re: [Intel-gfx] [PATCH] drm/i915: Remove the preliminary_hw_support shackles from CHV

[Daniel Vetter]

On Wed, Mar 11, 2015 at 10:52:28PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> CHV should be in a good enough shape now, so let's drop the
> .is_preliminary flag.
> 
> Signed-off-by: Ville Syrjälä 

Yay! Queued for -next, thanks for the patch.
-Daniel
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 15f58d0..82f8be4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -346,7 +346,6 @@ static const struct intel_device_info 
> intel_broadwell_gt3m_info = {
>  };
>  
>  static const struct intel_device_info intel_cherryview_info = {
> - .is_preliminary = 1,
>   .gen = 8, .num_pipes = 3,
>   .need_gfx_hws = 1, .has_hotplug = 1,
>   .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> -- 
> 2.0.5
> 
> ___
> 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: redefine WARN_ON_ONCE to include the condition

[Jani Nikula]

Same as

commit c883ef1b1c998d2d66866772fd0fc34afa45641e
Author: Mika Kuoppala 
Date:   Tue Oct 28 17:32:30 2014 +0200

drm/i915: Redefine WARN_ON to include the condition

but for WARN_ON_ONCE. Since the kernel WARN_ON_ONCE actually picks up
*our* version of WARN_ON, we end up with messages like

[  838.285319] WARN_ON(!__warned)

which are not that helpful.

Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/i915_drv.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a80b15f7acfa..a76c6eee593c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -70,6 +70,9 @@
 #define WARN_ON(x) WARN((x), "WARN_ON(" #x ")")
 #endif
 
+#undef WARN_ON_ONCE
+#define WARN_ON_ONCE(x) WARN_ONCE((x), "WARN_ON_ONCE(" #x ")")
+
 #define MISSING_CASE(x) WARN(1, "Missing switch case (%lu) in %s\n", \
 (long) (x), __func__);
 
-- 
2.1.4

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

[Intel-gfx] [PATCH v2] drm/i915: Do not use ggtt_view with (aliasing) PPGTT

[Joonas Lahtinen]

GGTT views are only applicable when dealing with GGTT. Change the code to
reject ggtt_view where it should not be used and require it when it should
be.

v2:
- Dropped _ppgtt_ infixes, allow both types to be passed
- Disregard other but normal views when no view is specified
- More checks that valid parameters are passed
- More readable error checking

Signed-off-by: Joonas Lahtinen 
---
 drivers/gpu/drm/i915/i915_drv.h | 102 +-
 drivers/gpu/drm/i915/i915_gem.c | 169 +++-
 drivers/gpu/drm/i915/i915_gem_gtt.c |  67 ++
 3 files changed, 219 insertions(+), 119 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a80b15f..8b7d4f9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2620,20 +2620,16 @@ void i915_gem_vma_destroy(struct i915_vma *vma);
 #define PIN_GLOBAL 0x4
 #define PIN_OFFSET_BIAS 0x8
 #define PIN_OFFSET_MASK (~4095)
-int __must_check i915_gem_object_pin_view(struct drm_i915_gem_object *obj,
- struct i915_address_space *vm,
- uint32_t alignment,
- uint64_t flags,
- const struct i915_ggtt_view *view);
-static inline
-int __must_check i915_gem_object_pin(struct drm_i915_gem_object *obj,
-struct i915_address_space *vm,
-uint32_t alignment,
-uint64_t flags)
-{
-   return i915_gem_object_pin_view(obj, vm, alignment, flags,
-   &i915_ggtt_view_normal);
-}
+int __must_check
+i915_gem_object_pin(struct drm_i915_gem_object *obj,
+   struct i915_address_space *vm,
+   uint32_t alignment,
+   uint64_t flags);
+int __must_check
+i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
+const struct i915_ggtt_view *view,
+uint32_t alignment,
+uint64_t flags);
 
 int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
  u32 flags);
@@ -2797,60 +2793,48 @@ struct dma_buf *i915_gem_prime_export(struct drm_device 
*dev,
 
 void i915_gem_restore_fences(struct drm_device *dev);
 
-unsigned long i915_gem_obj_offset_view(struct drm_i915_gem_object *o,
-  struct i915_address_space *vm,
-  enum i915_ggtt_view_type view);
-static inline
-unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o,
- struct i915_address_space *vm)
+static inline bool i915_is_ggtt(struct i915_address_space *vm);
+
+unsigned long
+i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o,
+ enum i915_ggtt_view_type view);
+unsigned long
+i915_gem_obj_offset(struct drm_i915_gem_object *o,
+   struct i915_address_space *vm);
+static inline unsigned long
+i915_gem_obj_ggtt_offset(struct drm_i915_gem_object *o)
 {
-   return i915_gem_obj_offset_view(o, vm, I915_GGTT_VIEW_NORMAL);
+   return i915_gem_obj_ggtt_offset_view(o, I915_GGTT_VIEW_NORMAL);
 }
+
 bool i915_gem_obj_bound_any(struct drm_i915_gem_object *o);
-bool i915_gem_obj_bound_view(struct drm_i915_gem_object *o,
-struct i915_address_space *vm,
-enum i915_ggtt_view_type view);
-static inline
+bool i915_gem_obj_ggtt_bound_view(struct drm_i915_gem_object *o,
+ enum i915_ggtt_view_type view);
 bool i915_gem_obj_bound(struct drm_i915_gem_object *o,
-   struct i915_address_space *vm)
-{
-   return i915_gem_obj_bound_view(o, vm, I915_GGTT_VIEW_NORMAL);
-}
+   struct i915_address_space *vm);
 
 unsigned long i915_gem_obj_size(struct drm_i915_gem_object *o,
struct i915_address_space *vm);
-struct i915_vma *i915_gem_obj_to_vma_view(struct drm_i915_gem_object *obj,
- struct i915_address_space *vm,
- const struct i915_ggtt_view *view);
-static inline
-struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
-struct i915_address_space *vm)
-{
-   return i915_gem_obj_to_vma_view(obj, vm, &i915_ggtt_view_normal);
-}
-
 struct i915_vma *
-i915_gem_obj_lookup_or_create_vma_view(struct drm_i915_gem_object *obj,
-  struct i915_address_space *vm,
-  const struct i915_ggtt_view *view);
+i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
+   struct i915_address_space *vm);
+struct i915_vma *
+i915_gem_obj_to_ggtt_view(struct drm_i915_gem_object *obj,
+ 

[Intel-gfx] [PATCH v3] drm/i915: Optimistically spin for the request completion

[Chris Wilson]

This provides a nice boost to mesa in swap bound scenarios (as mesa
throttles itself to the previous frame and given the scenario that will
complete shortly). It will also provide a good boost to systems running
with semaphores disabled and so frequently waiting on the GPU as it
switches rings. In the most favourable of microbenchmarks, this can
increase performance by around 15% - though in practice improvements
will be marginal and rarely noticeable.

v2: Account for user timeouts
v3: Limit the spinning to a single jiffie (~1us) at most. On an
otherwise idle system, there is no scheduler contention and so without a
limit we would spin until the GPU is ready.

Signed-off-by: Chris Wilson 
Cc: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_gem.c | 48 +++--
 1 file changed, 42 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1cc46558bf2a..df96f9704cf2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1191,6 +1191,33 @@ static bool missed_irq(struct drm_i915_private *dev_priv,
return test_bit(ring->id, &dev_priv->gpu_error.missed_irq_rings);
 }
 
+static int __i915_spin_request(struct drm_i915_gem_request *req)
+{
+   struct intel_engine_cs *ring = i915_gem_request_get_ring(req);
+   struct drm_i915_private *dev_priv = to_i915(ring->dev);
+   unsigned long timeout;
+   int ret = -EBUSY;
+
+   intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+   timeout = jiffies + 1;
+   while (!need_resched()) {
+   if (i915_gem_request_completed(req, true)) {
+   ret = 0;
+   goto out;
+   }
+
+   if (time_after_eq(jiffies, timeout))
+   break;
+
+   cpu_relax_lowlatency();
+   }
+   if (i915_gem_request_completed(req, false))
+   ret = 0;
+out:
+   intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
+   return ret;
+}
+
 /**
  * __i915_wait_request - wait until execution of request has finished
  * @req: duh!
@@ -1235,12 +1262,20 @@ int __i915_wait_request(struct drm_i915_gem_request 
*req,
if (ring->id == RCS && INTEL_INFO(dev)->gen >= 6)
gen6_rps_boost(dev_priv, file_priv);
 
-   if (!irq_test_in_progress && WARN_ON(!ring->irq_get(ring)))
-   return -ENODEV;
-
/* Record current time in case interrupted by signal, or wedged */
trace_i915_gem_request_wait_begin(req);
before = ktime_get_raw_ns();
+
+   /* Optimistic spin for the next jiffie before touching IRQs */
+   ret = __i915_spin_request(req);
+   if (ret == 0)
+   goto out;
+
+   if (!irq_test_in_progress && WARN_ON(!ring->irq_get(ring))) {
+   ret = -ENODEV;
+   goto out;
+   }
+
for (;;) {
struct timer_list timer;
 
@@ -1289,14 +1324,15 @@ int __i915_wait_request(struct drm_i915_gem_request 
*req,
destroy_timer_on_stack(&timer);
}
}
-   now = ktime_get_raw_ns();
-   trace_i915_gem_request_wait_end(req);
-
if (!irq_test_in_progress)
ring->irq_put(ring);
 
finish_wait(&ring->irq_queue, &wait);
 
+out:
+   now = ktime_get_raw_ns();
+   trace_i915_gem_request_wait_end(req);
+
if (timeout) {
s64 tres = *timeout - (now - before);
 
-- 
2.1.4

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

Re: [Intel-gfx] [PATCH v3] drm/i915: Optimistically spin for the request completion

[Chris Wilson]

On Thu, Mar 12, 2015 at 11:11:17AM +, Chris Wilson wrote:
> This provides a nice boost to mesa in swap bound scenarios (as mesa
> throttles itself to the previous frame and given the scenario that will
> complete shortly). It will also provide a good boost to systems running
> with semaphores disabled and so frequently waiting on the GPU as it
> switches rings. In the most favourable of microbenchmarks, this can
> increase performance by around 15% - though in practice improvements
> will be marginal and rarely noticeable.
> 
> v2: Account for user timeouts
> v3: Limit the spinning to a single jiffie (~1us) at most. On an
> otherwise idle system, there is no scheduler contention and so without a
> limit we would spin until the GPU is ready.
> 
> Signed-off-by: Chris Wilson 
> Cc: Daniel Vetter 

I'm really happy with this tradeoff. Can we throw this to the vultures
to see if anyone notices?
-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 i-g-t 2/2] tests/pm_sseu: Create new test pm_sseu

[Thomas Wood]

On 10 March 2015 at 21:17,   wrote:
> From: Jeff McGee 
>
> New test pm_sseu is intended for any subtest related to the
> slice/subslice/EU power gating feature. The sole initial subtest,
> 'full-enable', confirms that the slice/subslice/EU state is at
> full enablement when the render engine is active. Starting with
> Gen9 SKL, the render power gating feature can leave SSEU in a
> partially enabled state upon resumption of render work unless
> explicit action is taken.

Please add a short description to the test using the
IGT_TEST_DESCRIPTION macro, so that it is included in the
documentation and help output.

>
> Signed-off-by: Jeff McGee 
> ---
>  tests/.gitignore   |   1 +
>  tests/Makefile.sources |   1 +
>  tests/pm_sseu.c| 373 
> +
>  3 files changed, 375 insertions(+)
>  create mode 100644 tests/pm_sseu.c
>
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 7b4dd94..23094ce 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -144,6 +144,7 @@ pm_psr
>  pm_rc6_residency
>  pm_rpm
>  pm_rps
> +pm_sseu
>  prime_nv_api
>  prime_nv_pcopy
>  prime_nv_test
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 51e8376..74106c0 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -82,6 +82,7 @@ TESTS_progs_M = \
> pm_rpm \
> pm_rps \
> pm_rc6_residency \
> +   pm_sseu \
> prime_self_import \
> template \
> $(NULL)
> diff --git a/tests/pm_sseu.c b/tests/pm_sseu.c
> new file mode 100644
> index 000..45aeef3
> --- /dev/null
> +++ b/tests/pm_sseu.c
> @@ -0,0 +1,373 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *Jeff McGee 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "drmtest.h"
> +#include "i915_drm.h"
> +#include "intel_io.h"
> +#include "intel_bufmgr.h"
> +#include "intel_batchbuffer.h"
> +#include "intel_chipset.h"
> +#include "ioctl_wrappers.h"
> +#include "igt_debugfs.h"
> +#include "media_spin.h"
> +
> +static double
> +to_dt(const struct timespec *start, const struct timespec *end)
> +{
> +   double dt;
> +
> +   dt = (end->tv_sec - start->tv_sec) * 1e3;
> +   dt += (end->tv_nsec - start->tv_nsec) * 1e-6;
> +
> +   return dt;
> +}
> +
> +struct status {
> +   struct {
> +   int slice_total;
> +   int subslice_total;
> +   int subslice_per;
> +   int eu_total;
> +   int eu_per;
> +   bool has_slice_pg;
> +   bool has_subslice_pg;
> +   bool has_eu_pg;
> +   } info;
> +   struct {
> +   int slice_total;
> +   int subslice_total;
> +   int subslice_per;
> +   int eu_total;
> +   int eu_per;
> +   } hw;
> +};
> +
> +#define DBG_STATUS_BUF_SIZE 4096
> +
> +struct {
> +   int init;
> +   int status_fd;
> +   char status_buf[DBG_STATUS_BUF_SIZE];
> +} dbg;
> +
> +static void
> +dbg_get_status_section(const char *title, char **first, char **last)
> +{
> +   char *pos;
> +
> +   *first = strstr(dbg.status_buf, title);
> +   igt_assert(*first != NULL);
> +
> +   pos = *first;
> +   do {
> +   pos = strchr(pos, '\n');
> +   igt_assert(pos != NULL);
> +   pos++;
> +   } while (*pos == ' '); /* lines in the section begin with a space */
> +   *last = pos - 1;
> +}
> +
> +static int
> +dbg_get_int(const char *first, const char *last, const char *name)
> +{
> +   char *pos;
> +
> +   pos = strstr(first, name);
> +   igt_assert(pos != NULL);
> +   pos = strstr(pos, ":");
> +   igt_assert(pos != NULL);
> +   pos += 2;
> +   igt_assert(pos < las

Re: [Intel-gfx] [PATCH v2] drm/i915: Do not use ggtt_view with (aliasing) PPGTT

[Tvrtko Ursulin]

Hi,

Much more likeable in general, some comments inline:

On 03/12/2015 11:10 AM, Joonas Lahtinen wrote:

GGTT views are only applicable when dealing with GGTT. Change the code to
reject ggtt_view where it should not be used and require it when it should
be.

v2:
- Dropped _ppgtt_ infixes, allow both types to be passed
- Disregard other but normal views when no view is specified
- More checks that valid parameters are passed
- More readable error checking

Signed-off-by: Joonas Lahtinen 
---
  drivers/gpu/drm/i915/i915_drv.h | 102 +-
  drivers/gpu/drm/i915/i915_gem.c | 169 +++-
  drivers/gpu/drm/i915/i915_gem_gtt.c |  67 ++
  3 files changed, 219 insertions(+), 119 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a80b15f..8b7d4f9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2620,20 +2620,16 @@ void i915_gem_vma_destroy(struct i915_vma *vma);
  #define PIN_GLOBAL 0x4
  #define PIN_OFFSET_BIAS 0x8
  #define PIN_OFFSET_MASK (~4095)
-int __must_check i915_gem_object_pin_view(struct drm_i915_gem_object *obj,
- struct i915_address_space *vm,
- uint32_t alignment,
- uint64_t flags,
- const struct i915_ggtt_view *view);
-static inline
-int __must_check i915_gem_object_pin(struct drm_i915_gem_object *obj,
-struct i915_address_space *vm,
-uint32_t alignment,
-uint64_t flags)
-{
-   return i915_gem_object_pin_view(obj, vm, alignment, flags,
-   &i915_ggtt_view_normal);
-}
+int __must_check
+i915_gem_object_pin(struct drm_i915_gem_object *obj,
+   struct i915_address_space *vm,
+   uint32_t alignment,
+   uint64_t flags);
+int __must_check
+i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
+const struct i915_ggtt_view *view,
+uint32_t alignment,
+uint64_t flags);

  int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
  u32 flags);
@@ -2797,60 +2793,48 @@ struct dma_buf *i915_gem_prime_export(struct drm_device 
*dev,

  void i915_gem_restore_fences(struct drm_device *dev);

-unsigned long i915_gem_obj_offset_view(struct drm_i915_gem_object *o,
-  struct i915_address_space *vm,
-  enum i915_ggtt_view_type view);
-static inline
-unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o,
- struct i915_address_space *vm)
+static inline bool i915_is_ggtt(struct i915_address_space *vm);
+
+unsigned long
+i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o,
+ enum i915_ggtt_view_type view);
+unsigned long
+i915_gem_obj_offset(struct drm_i915_gem_object *o,
+   struct i915_address_space *vm);
+static inline unsigned long
+i915_gem_obj_ggtt_offset(struct drm_i915_gem_object *o)
  {
-   return i915_gem_obj_offset_view(o, vm, I915_GGTT_VIEW_NORMAL);
+   return i915_gem_obj_ggtt_offset_view(o, I915_GGTT_VIEW_NORMAL);
  }
+
  bool i915_gem_obj_bound_any(struct drm_i915_gem_object *o);
-bool i915_gem_obj_bound_view(struct drm_i915_gem_object *o,
-struct i915_address_space *vm,
-enum i915_ggtt_view_type view);
-static inline
+bool i915_gem_obj_ggtt_bound_view(struct drm_i915_gem_object *o,
+ enum i915_ggtt_view_type view);
  bool i915_gem_obj_bound(struct drm_i915_gem_object *o,
-   struct i915_address_space *vm)
-{
-   return i915_gem_obj_bound_view(o, vm, I915_GGTT_VIEW_NORMAL);
-}
+   struct i915_address_space *vm);

  unsigned long i915_gem_obj_size(struct drm_i915_gem_object *o,
struct i915_address_space *vm);
-struct i915_vma *i915_gem_obj_to_vma_view(struct drm_i915_gem_object *obj,
- struct i915_address_space *vm,
- const struct i915_ggtt_view *view);
-static inline
-struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
-struct i915_address_space *vm)
-{
-   return i915_gem_obj_to_vma_view(obj, vm, &i915_ggtt_view_normal);
-}
-
  struct i915_vma *
-i915_gem_obj_lookup_or_create_vma_view(struct drm_i915_gem_object *obj,
-  struct i915_address_space *vm,
-  const struct i915_ggtt_view *view);
+i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
+

Re: [Intel-gfx] [PATCH 05/23] drm/i915: Allocate a drm_atomic_state for the legacy modeset code

[Ander Conselvan De Oliveira]

On Mon, 2015-03-09 at 16:19 -0700, Matt Roper wrote:
> On Wed, Mar 04, 2015 at 04:33:17PM +0100, Daniel Vetter wrote:
> > On Tue, Mar 03, 2015 at 03:21:59PM +0200, Ander Conselvan de Oliveira wrote:
> > > For the atomic conversion, the mode set paths need to be changed to rely
> > > on an atomic state instead of using the staged config. By using an
> > > atomic state for the legacy code, we will be able to convert the code
> > > base in small chunks.
> > > 
> > > Signed-off-by: Ander Conselvan de Oliveira 
> > > 
> > 
> > Two small comments below.
> > -Daniel
> > 
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 118 
> > > +++
> > >  1 file changed, 91 insertions(+), 27 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index 798de7b..97d4df5 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -37,6 +37,7 @@
> > >  #include 
> > >  #include "i915_drv.h"
> > >  #include "i915_trace.h"
> > > +#include 
> > >  #include 
> > >  #include 
> > >  #include 
> > > @@ -10290,10 +10291,22 @@ static bool check_digital_port_conflicts(struct 
> > > drm_device *dev)
> > >   return true;
> > >  }
> > >  
> > > -static struct intel_crtc_state *
> > > +static void
> > > +clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
> > > +{
> > > + struct drm_crtc_state tmp_state;
> > > +
> > > + /* Clear only the intel specific part of the crtc state */
> > > + tmp_state = crtc_state->base;
> > > + memset(crtc_state, 0, sizeof *crtc_state);
> > > + crtc_state->base = tmp_state;
> > > +}
> > 
> > I guess this is to clear out state which we want to recompute, and our
> > compute_config code assumes that it's always kzalloc'ed a new config?
> > 
> > I think this should be part of the crtc duplicate_state callback to make
> > sure we're doing this consistently.
> 
> If we zero out the config in duplicate_state(), then we're not really
> "duplicating" anymore are we?  Ultimately we want to be able to
> duplicate the existing state, tweak a couple things, and then pass that
> state through the atomic pipeline, so it feels like doing a clear in the
> duplicate function is the wrong move, even if it would work for the
> frankenstein-style codebase we have at the moment.

Yeah, I guess I can't run away from inspecting the code and fixing
whatever expects the crtc_state to be zeroed.

Ander

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

Re: [Intel-gfx] [PATCH] drm/i915: Ensure plane->state->fb stays in sync with plane->fb

[Jani Nikula]

Matt, please review or suggest an alternative for v4.0-rc.

Thanks,
Jani.


On Thu, 12 Mar 2015, Xi Ruoyao  wrote:
> plane->state->fb and plane->fb should always reference the same FB so
> that atomic and legacy codepaths have the same view of display state.
> However, there are some places in kernel code that directly set
> plane->fb and neglect to update plane->state->fb. If we never do a
> successful update through the atomic pipeline, the RmFB cleanup code
> will look at the plane->state->fb pointer, which has never actually
> been set to a legitimate value, and try to clean it up, leading to
> BUG's.
>
> Add a quick helper function to synchronize plane->state->fb with
> plane->fb and call it everywhere the driver tries to manually set
> plane->fb outside of the atomic pipeline. In this function, use
> drm_atomic_set_fb_for_plane instead of writing plane->state->fb
> directly to keep the reference count right.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88909
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=93711
> Signed-off-by: Matt Roper 
> Signed-off-by: Daniel Vetter 
> Signed-off-by: Xi Ruoyao 
> ---
>  This is modified from Matt Roper's patch to drm-intel-nightly with 
>  commit id
>
>   afd65eb4cc0578a9c07d621acdb8a570e2782bf7
>
>  However this bug exists in mainline kernel too, so I created this to
>  fix it in mainline kernel.
>
>  A minor change is to use drm_atomic_set_fb_for_plane instead of
>  update reference count manually.
>
>  drivers/gpu/drm/i915/intel_display.c | 17 +
>  1 file changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index e730789..c483f2f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -37,6 +37,7 @@
>  #include 
>  #include "i915_drv.h"
>  #include "i915_trace.h"
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -2416,6 +2417,14 @@ out_unref_obj:
>   return false;
>  }
>  
> +/* Update plane->state->fb to match plane->fb after driver-internal updates 
> */
> +static void
> +update_state_fb(struct drm_plane *plane)
> +{
> + if (plane->fb != plane->state->fb)
> + drm_atomic_set_fb_for_plane(plane->state, plane->fb);
> +}
> +
>  static void
>  intel_find_plane_obj(struct intel_crtc *intel_crtc,
>struct intel_initial_plane_config *plane_config)
> @@ -2462,6 +2471,8 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc,
>   break;
>   }
>   }
> +
> + update_state_fb(intel_crtc->base.primary);
>  }
>  
>  static void i9xx_update_primary_plane(struct drm_crtc *crtc,
> @@ -6650,6 +6661,7 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
> plane_config->size);
>  
>   crtc->base.primary->fb = fb;
> + update_state_fb(crtc->base.primary);
>  }
>  
>  static void chv_crtc_clock_get(struct intel_crtc *crtc,
> @@ -7687,6 +7699,7 @@ skylake_get_initial_plane_config(struct intel_crtc 
> *crtc,
> plane_config->size);
>  
>   crtc->base.primary->fb = fb;
> + update_state_fb(crtc->base.primary);
>   return;
>  
>  error:
> @@ -7778,6 +7791,7 @@ ironlake_get_initial_plane_config(struct intel_crtc 
> *crtc,
> plane_config->size);
>  
>   crtc->base.primary->fb = fb;
> + update_state_fb(crtc->base.primary);
>  }
>  
>  static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> @@ -9816,6 +9830,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>   drm_gem_object_reference(&obj->base);
>  
>   crtc->primary->fb = fb;
> + update_state_fb(crtc->primary);
>  
>   work->pending_flip_obj = obj;
>  
> @@ -9884,6 +9899,7 @@ cleanup_unpin:
>  cleanup_pending:
>   atomic_dec(&intel_crtc->unpin_work_count);
>   crtc->primary->fb = old_fb;
> + update_state_fb(crtc->primary);
>   drm_gem_object_unreference(&work->old_fb_obj->base);
>   drm_gem_object_unreference(&obj->base);
>   mutex_unlock(&dev->struct_mutex);
> @@ -13718,6 +13734,7 @@ void intel_modeset_gem_init(struct drm_device *dev)
> to_intel_crtc(c)->pipe);
>   drm_framebuffer_unreference(c->primary->fb);
>   c->primary->fb = NULL;
> + update_state_fb(c->primary);
>   }
>   }
>   mutex_unlock(&dev->struct_mutex);
> -- 
> 1.9.1
>

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

[Intel-gfx] [PATCH] drm/i915: Ensure plane->state->fb stays in sync with plane->fb

[Xi Ruoyao]

plane->state->fb and plane->fb should always reference the same FB so
that atomic and legacy codepaths have the same view of display state.
However, there are some places in kernel code that directly set
plane->fb and neglect to update plane->state->fb. If we never do a
successful update through the atomic pipeline, the RmFB cleanup code
will look at the plane->state->fb pointer, which has never actually
been set to a legitimate value, and try to clean it up, leading to
BUG's.

Add a quick helper function to synchronize plane->state->fb with
plane->fb and call it everywhere the driver tries to manually set
plane->fb outside of the atomic pipeline. In this function, use
drm_atomic_set_fb_for_plane instead of writing plane->state->fb
directly to keep the reference count right.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88909
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=93711
Signed-off-by: Matt Roper 
Signed-off-by: Daniel Vetter 
Signed-off-by: Xi Ruoyao 
---
 This is modified from Matt Roper's patch to drm-intel-nightly with 
 commit id

afd65eb4cc0578a9c07d621acdb8a570e2782bf7

 However this bug exists in mainline kernel too, so I created this to
 fix it in mainline kernel.

 A minor change is to use drm_atomic_set_fb_for_plane instead of
 update reference count manually.

 drivers/gpu/drm/i915/intel_display.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index e730789..c483f2f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -37,6 +37,7 @@
 #include 
 #include "i915_drv.h"
 #include "i915_trace.h"
+#include 
 #include 
 #include 
 #include 
@@ -2416,6 +2417,14 @@ out_unref_obj:
return false;
 }
 
+/* Update plane->state->fb to match plane->fb after driver-internal updates */
+static void
+update_state_fb(struct drm_plane *plane)
+{
+   if (plane->fb != plane->state->fb)
+   drm_atomic_set_fb_for_plane(plane->state, plane->fb);
+}
+
 static void
 intel_find_plane_obj(struct intel_crtc *intel_crtc,
 struct intel_initial_plane_config *plane_config)
@@ -2462,6 +2471,8 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc,
break;
}
}
+
+   update_state_fb(intel_crtc->base.primary);
 }
 
 static void i9xx_update_primary_plane(struct drm_crtc *crtc,
@@ -6650,6 +6661,7 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
  plane_config->size);
 
crtc->base.primary->fb = fb;
+   update_state_fb(crtc->base.primary);
 }
 
 static void chv_crtc_clock_get(struct intel_crtc *crtc,
@@ -7687,6 +7699,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
  plane_config->size);
 
crtc->base.primary->fb = fb;
+   update_state_fb(crtc->base.primary);
return;
 
 error:
@@ -7778,6 +7791,7 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc,
  plane_config->size);
 
crtc->base.primary->fb = fb;
+   update_state_fb(crtc->base.primary);
 }
 
 static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
@@ -9816,6 +9830,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
drm_gem_object_reference(&obj->base);
 
crtc->primary->fb = fb;
+   update_state_fb(crtc->primary);
 
work->pending_flip_obj = obj;
 
@@ -9884,6 +9899,7 @@ cleanup_unpin:
 cleanup_pending:
atomic_dec(&intel_crtc->unpin_work_count);
crtc->primary->fb = old_fb;
+   update_state_fb(crtc->primary);
drm_gem_object_unreference(&work->old_fb_obj->base);
drm_gem_object_unreference(&obj->base);
mutex_unlock(&dev->struct_mutex);
@@ -13718,6 +13734,7 @@ void intel_modeset_gem_init(struct drm_device *dev)
  to_intel_crtc(c)->pipe);
drm_framebuffer_unreference(c->primary->fb);
c->primary->fb = NULL;
+   update_state_fb(c->primary);
}
}
mutex_unlock(&dev->struct_mutex);
-- 
1.9.1

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

[Intel-gfx] [PATCH 1/1] drm/i915: Fix BUG in i915_gem.c when switch to console

[Xi Ruoyao]

In intel_crtc_page_flip, intel_display.c, the code changed the framebuffer
assigned to plane crtc->primary by

crtc->primary->fb = fb;

However, it forgot to change crtc->primary->state->fb. However, when we
switch to console, some kernel code will read crtc->primary->state->fb
to get the framebuffer assigned to crtc->primaty. Then a framebuffer
object can be unpinned twice and a kernel BUG will be produced in 
i915_gem.c.


So, update crtc->primary->state->fb in intel_display.c using
drm_atomic_set_fb_for_plane to fix the BUG.

Signed-off-by: Xi Ruoyao 
---
 drivers/gpu/drm/i915/intel_display.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c

index e730789..97083fd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -37,6 +37,7 @@
 #include 
 #include "i915_drv.h"
 #include "i915_trace.h"
+#include 
 #include 
 #include 
 #include 
@@ -9816,6 +9817,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 drm_gem_object_reference(&obj->base);

 crtc->primary->fb = fb;
+drm_atomic_set_fb_for_plane(crtc->primary->state, fb);

 work->pending_flip_obj = obj;

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

Re: [Intel-gfx] [PATCH 1/2] gpio/crystalcove: Export Panel and backlight en/disable signals as GPIO

[Linus Walleij]

On Fri, Mar 6, 2015 at 5:23 PM, Kumar, Shobhit  wrote:

> There are actually two lines for Panel Power control and Backlight
> enable/disable. I have already moved towards adding a new Cell device
> for PWM child device and a new pwm driver for the same. That will take
> care of backlight thingy. But for the Panel power control, I am at loss
> for how best to program that.

Isn't it just a very simple regulator (just on/off of fixed voltage) cell?

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

Re: [Intel-gfx] [PATCH v2] drm/i915: Fix BUG in i915_gem.c when switch to console

[Xi Ruoyao]

On 03/12/2015 04:28 PM, Jani Nikula wrote:

On Thu, 12 Mar 2015, Xi Ruoyao  wrote:

In intel_crtc_page_flip, intel_display.c, the code changed the framebuffer
assigned to plane crtc->primary by

crtc->primary->fb = fb;

However, it forgot to change crtc->primary->state->fb. However, when we
switch to console, some kernel code will read crtc->primary->state->fb
to get the framebuffer assigned to crtc->primaty. Then a framebuffer
object can be unpinned twice and a kernel BUG will be produced in i915_gem.c.

So, update crtc->primary->state->fb in intel_display.c using
drm_atomic_set_fb_for_plane to fix the BUG.

Signed-off-by: Xi Ruoyao 
Fixed: Bug 93711 

Is this a problem with drm-intel-nightly? In particular see

commit afd65eb4cc0578a9c07d621acdb8a570e2782bf7
Author: Matt Roper 
Date:   Tue Feb 3 13:10:04 2015 -0800

 drm/i915: Ensure plane->state->fb stays in sync with plane->fb

Matt, do you think this fixes the described issue? Can we backport to
drm-intel-fixes (and v4.0)?

BR,
Jani.




---
  Sorry, the previous patch is mangled by email client, so I am re-sending it.

  drivers/gpu/drm/i915/intel_display.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index e730789..97083fd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -37,6 +37,7 @@
  #include 
  #include "i915_drv.h"
  #include "i915_trace.h"
+#include 
  #include 
  #include 
  #include 
@@ -9816,6 +9817,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
drm_gem_object_reference(&obj->base);
  
  	crtc->primary->fb = fb;

+   drm_atomic_set_fb_for_plane(crtc->primary->state, fb);
  
  	work->pending_flip_obj = obj;
  
--

1.9.1


Thank you to read my patch.

Matt's patch is better than mine since it fixed more conditions causing the
unsynchronize of plane->fb and plane->fb. But unfortunately, it cannot be
applied to mainline kernel (Linux 4.0.0-rc3) directly.

I'll try to apply Matt's changes to the mainline kernel.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Spelling s/auxilliary/auxiliary/

[Geert Uytterhoeven]

Signed-off-by: Geert Uytterhoeven 
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 6d8e29abbc333c87..74fdd8ca2cb2a347 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1420,7 +1420,7 @@ void intel_power_domains_init_hw(struct drm_i915_private 
*dev_priv)
 }
 
 /**
- * intel_aux_display_runtime_get - grab an auxilliary power domain reference
+ * intel_aux_display_runtime_get - grab an auxiliary power domain reference
  * @dev_priv: i915 device instance
  *
  * This function grabs a power domain reference for the auxiliary power domain
@@ -1437,10 +1437,10 @@ void intel_aux_display_runtime_get(struct 
drm_i915_private *dev_priv)
 }
 
 /**
- * intel_aux_display_runtime_put - release an auxilliary power domain reference
+ * intel_aux_display_runtime_put - release an auxiliary power domain reference
  * @dev_priv: i915 device instance
  *
- * This function drops the auxilliary power domain reference obtained by
+ * This function drops the auxiliary power domain reference obtained by
  * intel_aux_display_runtime_get() and might power down the corresponding
  * hardware block right away if this is the last reference.
  */
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH 1/2] gpio/crystalcove: Export Panel and backlight en/disable signals as GPIO

[Linus Walleij]

On Wed, Feb 18, 2015 at 1:18 PM, Shobhit Kumar  wrote:

> Export Panel BACKLIGHT_EN(offset 0x51) and PANEL_EN(offset 0x52) as two
> additional GPIOs. Needed by display driver to enable the DSI panel on
> BYT platform where the Panel EN/Disable and Backlight control are
> routed thorugh CRC PMIC
>
> Cc: Linus Walleij 
> Cc: Alexandre Courbot 
> Cc: Thierry Reding 
> Signed-off-by: Shobhit Kumar 

This seems very unintuitive. I have a hard time believeing this:

> +#define GPIOPANELCTL   0x51

A special GPIO only designated to panel control? That is totally
counter to the *meaning* of "general purpose input/output".
It is not general purpose at all, it is special-purpose.

If the data sheet says a special-purpose pin is a "GPIO" when
it isn't the map of the world (the datasheet terminology) is wrong,
not the world, the usage is dedicated for the panel. Look
at the usecase, not the name.

Obviously that line is only for controlling the backlight, and you
should spin a new child device from the Inte SoC PMIC MFD hub
and put a driver for it in drivers/video/backlight or wherever this
goes.

Lee: do you agree?

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

[Intel-gfx] [PATCH v2] drm/i915: Fix BUG in i915_gem.c when switch to console

[Xi Ruoyao]

In intel_crtc_page_flip, intel_display.c, the code changed the framebuffer
assigned to plane crtc->primary by

crtc->primary->fb = fb;

However, it forgot to change crtc->primary->state->fb. However, when we
switch to console, some kernel code will read crtc->primary->state->fb
to get the framebuffer assigned to crtc->primaty. Then a framebuffer
object can be unpinned twice and a kernel BUG will be produced in i915_gem.c.

So, update crtc->primary->state->fb in intel_display.c using
drm_atomic_set_fb_for_plane to fix the BUG.

Signed-off-by: Xi Ruoyao 
Fixed: Bug 93711 
---
 Sorry, the previous patch is mangled by email client, so I am re-sending it.

 drivers/gpu/drm/i915/intel_display.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index e730789..97083fd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -37,6 +37,7 @@
 #include 
 #include "i915_drv.h"
 #include "i915_trace.h"
+#include 
 #include 
 #include 
 #include 
@@ -9816,6 +9817,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
drm_gem_object_reference(&obj->base);
 
crtc->primary->fb = fb;
+   drm_atomic_set_fb_for_plane(crtc->primary->state, fb);
 
work->pending_flip_obj = obj;
 
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH v3] drm/i915: Optimistically spin for the request completion

[Tvrtko Ursulin]

On 03/12/2015 11:11 AM, Chris Wilson wrote:

This provides a nice boost to mesa in swap bound scenarios (as mesa
throttles itself to the previous frame and given the scenario that will
complete shortly). It will also provide a good boost to systems running
with semaphores disabled and so frequently waiting on the GPU as it
switches rings. In the most favourable of microbenchmarks, this can
increase performance by around 15% - though in practice improvements
will be marginal and rarely noticeable.

v2: Account for user timeouts
v3: Limit the spinning to a single jiffie (~1us) at most. On an
otherwise idle system, there is no scheduler contention and so without a
limit we would spin until the GPU is ready.


Isn't one jiffie 1-10ms typically?

Regards,

Tvrtko

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

Re: [Intel-gfx] [PATCH v3] drm/i915: Optimistically spin for the request completion

[Chris Wilson]

On Thu, Mar 12, 2015 at 01:14:30PM +, Tvrtko Ursulin wrote:
> On 03/12/2015 11:11 AM, Chris Wilson wrote:
> >This provides a nice boost to mesa in swap bound scenarios (as mesa
> >throttles itself to the previous frame and given the scenario that will
> >complete shortly). It will also provide a good boost to systems running
> >with semaphores disabled and so frequently waiting on the GPU as it
> >switches rings. In the most favourable of microbenchmarks, this can
> >increase performance by around 15% - though in practice improvements
> >will be marginal and rarely noticeable.
> >
> >v2: Account for user timeouts
> >v3: Limit the spinning to a single jiffie (~1us) at most. On an
> >otherwise idle system, there is no scheduler contention and so without a
> >limit we would spin until the GPU is ready.
> 
> Isn't one jiffie 1-10ms typically?

1ms. I was just thinking of doing USECS_PER_SEC / HZ, then realised that
was a jiffie, hence the confusion. At any rate, it is still the minimum
we can trivially wait for (without an expensive hrtimer).
-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 i-g-t 06/15] lib: s/IGT_DEBUG_INTERACTIVE/--interactive-debug=var

[Damien Lespiau]

On Tue, Jan 20, 2015 at 11:36:03AM +0100, Daniel Vetter wrote:
> On Mon, Jan 12, 2015 at 10:21:58AM -0800, Rodrigo Vivi wrote:
> > Use cmdline variable for interactive debug instead of env var.
> > 
> > v2: Make interactive-debug domain optional and use "all" when not set.
> > 
> > Signed-off-by: Rodrigo Vivi 
> 
> I went ahead and applied this one to igt.

in igt_core.h:

  const char *igt_interactive_debug;

and not declared in any .c. Which means we'll define one such global
variable per compilation unit, not good :)

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

Re: [Intel-gfx] [PATCH i-g-t v3 00/13] Testing the Y tiled display

[Damien Lespiau]

On Tue, Mar 03, 2015 at 02:10:53PM +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin 
> 
> Starting with Skylake the display engine can scan out Y tiled objects. (Both
> legacy Y tiled, and the new Yf format.)
> 
> This series takes the original work by Damien Lespiau and converts it to use 
> the
> new frame buffer modifiers instead of object set/get tiling. Some patches 
> needed
> to be dropped, some added and some refactored.
> 
> v2: Refactored for fb modifier changes.
> 
> v3:
>* Addressing review comments.
>* Added Y(f) tiling sub tests to kms_flip_tiling.

Pushed this series (but patch 2). Thanks!

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

[Intel-gfx] [PATCH i-g-t] lib/fb: Use PRIx64 for uint64_t in format string

[Damien Lespiau]

Fix the following warning:

  igt_fb.c: In function 'igt_create_fb_with_bo_size':
  igt_fb.c:414:2: warning: format '%llx' expects argument of type
'long long unsigned int', but argument 9 has type 'uint64_t' [-Wformat=]

  igt_debug("%s(width=%d, height=%d, format=0x%x [bpp=%d], tiling=%llx, 
size=%d\n",

introduced by commit:

  commit e36091d1c7010e825897dc4487f9985ab353973b
  Author: Tvrtko Ursulin 
  Date:   Tue Mar 3 14:11:01 2015 +

  tiling: Convert framebuffer helpers to use fb modifiers

Cc: Tvrtko Ursulin 
Signed-off-by: Damien Lespiau 
---
 lib/igt_fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 5c92fac..ce5a102 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -411,7 +411,7 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
 
bpp = igt_drm_format_to_bpp(format);
 
-   igt_debug("%s(width=%d, height=%d, format=0x%x [bpp=%d], tiling=%llx, 
size=%d\n",
+   igt_debug("%s(width=%d, height=%d, format=0x%x [bpp=%d], 
tiling=0x%"PRIx64", size=%d\n",
  __func__, width, height, format, bpp, tiling, bo_size);
do_or_die(create_bo_for_fb(fd, width, height, bpp, tiling, bo_size,
   &fb->gem_handle, &fb->size, &fb->stride));
-- 
1.8.3.1

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

[Intel-gfx] [PATCH 11/13] drm/i915: Avoid overflowing the DP link rate arrays

[ville . syrjala]

From: Ville Syrjälä 

Complain loudly if we ever attempt to overflow the the supported_rates[]
array. This should never happen since the sink_rates[] array will always
be smaller or of equal size. But should someone change that we want to
catch it without scribblign over the stack.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_dp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 52376fd..a088186 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1200,6 +1200,8 @@ static int intersect_rates(const int *source_rates, int 
source_len,
 
while (i < source_len && j < sink_len) {
if (source_rates[i] == sink_rates[j]) {
+   if (WARN_ON(k >= DP_MAX_SUPPORTED_RATES))
+   return k;
supported_rates[k] = source_rates[i];
++k;
++i;
-- 
2.0.5

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

[Intel-gfx] [PATCH 03/13] drm/i915: Don't copy the DP source rates arrays

[ville . syrjala]

From: Ville Syrjälä 

The source rates don't change, so we can just point the caller at the
const arrays.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_dp.c | 24 ++--
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d638f5e..537f1d0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1157,22 +1157,18 @@ intel_read_sink_rates(struct intel_dp *intel_dp, int 
*sink_rates)
 }
 
 static int
-intel_read_source_rates(struct intel_dp *intel_dp, int *source_rates)
+intel_dp_source_rates(struct intel_dp *intel_dp, const int **source_rates)
 {
struct drm_device *dev = intel_dp_to_dev(intel_dp);
-   int i;
-   int max_default_rate;
 
-   if (INTEL_INFO(dev)->gen >= 9 && intel_dp->supported_rates[0]) {
-   for (i = 0; i < ARRAY_SIZE(gen9_rates); ++i)
-   source_rates[i] = gen9_rates[i];
-   } else {
-   /* Index of the max_link_bw supported + 1 */
-   max_default_rate = (intel_dp_max_link_bw(intel_dp) >> 3) + 1;
-   for (i = 0; i < max_default_rate; ++i)
-   source_rates[i] = default_rates[i];
+   if (INTEL_INFO(dev)->gen >= 9) {
+   *source_rates = gen9_rates;
+   return ARRAY_SIZE(gen9_rates);
}
-   return i;
+
+   *source_rates = default_rates;
+
+   return (intel_dp_max_link_bw(intel_dp) >> 3) + 1;
 }
 
 static void
@@ -1269,12 +1265,12 @@ intel_dp_compute_config(struct intel_encoder *encoder,
int link_avail, link_clock;
int sink_rates[8];
int supported_rates[8] = {0};
-   int source_rates[8];
+   const int *source_rates;
int source_len, sink_len, supported_len;
 
sink_len = intel_read_sink_rates(intel_dp, sink_rates);
 
-   source_len = intel_read_source_rates(intel_dp, source_rates);
+   source_len = intel_dp_source_rates(intel_dp, &source_rates);
 
supported_len = intel_supported_rates(source_rates, source_len,
sink_rates, sink_len, supported_rates);
-- 
2.0.5

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

[Intel-gfx] [PATCH 07/13] drm/i915: Hide the source vs. sink rate handling from intel_dp_compute_config()

[ville . syrjala]

From: Ville Syrjälä 

intel_dp_compute_config() only really needs to know the rates supported
by both source and sink, so hide the raw source and sink arrays from it.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_dp.c | 37 +++--
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 61538f4..a88f932 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1192,9 +1192,9 @@ intel_dp_set_clock(struct intel_encoder *encoder,
}
 }
 
-static int intel_supported_rates(const int *source_rates, int source_len,
-const int *sink_rates, int sink_len,
-int *supported_rates)
+static int intersect_rates(const int *source_rates, int source_len,
+  const int *sink_rates, int sink_len,
+  int *supported_rates)
 {
int i = 0, j = 0, k = 0;
 
@@ -1213,6 +1213,21 @@ static int intel_supported_rates(const int 
*source_rates, int source_len,
return k;
 }
 
+static int intel_supported_rates(struct intel_dp *intel_dp,
+int *supported_rates)
+{
+   struct drm_device *dev = intel_dp_to_dev(intel_dp);
+   const int *source_rates, *sink_rates;
+   int source_len, sink_len;
+
+   sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
+   source_len = intel_dp_source_rates(dev, &source_rates);
+
+   return intersect_rates(source_rates, source_len,
+  sink_rates, sink_len,
+  supported_rates);
+}
+
 static int rate_to_index(int find, const int *rates)
 {
int i = 0;
@@ -1243,17 +1258,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
int max_clock;
int bpp, mode_rate;
int link_avail, link_clock;
-   const int *sink_rates;
-   int supported_rates[8] = {0};
-   const int *source_rates;
-   int source_len, sink_len, supported_len;
-
-   sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
-
-   source_len = intel_dp_source_rates(dev, &source_rates);
+   int supported_rates[DP_MAX_SUPPORTED_RATES] = {};
+   int supported_len;
 
-   supported_len = intel_supported_rates(source_rates, source_len,
-   sink_rates, sink_len, supported_rates);
+   supported_len = intel_supported_rates(intel_dp, supported_rates);
 
/* No common link rates between source and sink */
WARN_ON(supported_len <= 0);
@@ -1352,7 +1360,8 @@ found:
 
if (INTEL_INFO(dev)->gen >= 9 && intel_dp->supported_rates[0]) {
intel_dp->rate_select =
-   rate_to_index(supported_rates[clock], sink_rates);
+   rate_to_index(supported_rates[clock],
+ intel_dp->supported_rates);
intel_dp->link_bw = 0;
}
 
-- 
2.0.5

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

[Intel-gfx] [PATCH 10/13] drm/i915: Fix MST link rate handling

[ville . syrjala]

From: Ville Syrjälä 

Now that intel_dp_max_link_bw() no longer considers the source
restrictions we may try to enable MST with 5.4GHz even when the source
doesn't support it. To fix that switch the code over to handle the link
rate in the same way as the SST code handles it.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_dp.c | 12 
 drivers/gpu/drm/i915/intel_dp_mst.c | 16 +---
 drivers/gpu/drm/i915/intel_drv.h|  2 +-
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 28d9249..52376fd 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -122,8 +122,8 @@ static void vlv_init_panel_power_sequencer(struct intel_dp 
*intel_dp);
 static void vlv_steal_power_sequencer(struct drm_device *dev,
  enum pipe pipe);
 
-int
-intel_dp_max_link_bw(struct intel_dp *intel_dp)
+static int
+intel_dp_max_link_bw(struct intel_dp  *intel_dp)
 {
int max_link_bw = intel_dp->dpcd[DP_MAX_LINK_RATE];
 
@@ -1252,6 +1252,11 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp)
return rates[rate_to_index(0, rates) - 1];
 }
 
+int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
+{
+   return rate_to_index(rate, intel_dp->supported_rates);
+}
+
 bool
 intel_dp_compute_config(struct intel_encoder *encoder,
struct intel_crtc_state *pipe_config)
@@ -1371,8 +1376,7 @@ found:
if (intel_dp->num_supported_rates) {
intel_dp->link_bw = 0;
intel_dp->rate_select =
-   rate_to_index(supported_rates[clock],
- intel_dp->supported_rates);
+   intel_dp_rate_select(intel_dp, supported_rates[clock]);
} else {
intel_dp->link_bw =
drm_dp_link_rate_to_bw_code(supported_rates[clock]);
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
b/drivers/gpu/drm/i915/intel_dp_mst.c
index be12492..7e6f1259 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -38,7 +38,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder 
*encoder,
struct intel_dp *intel_dp = &intel_dig_port->dp;
struct drm_device *dev = encoder->base.dev;
int bpp;
-   int lane_count, slots;
+   int lane_count, slots, rate;
struct drm_display_mode *adjusted_mode = 
&pipe_config->base.adjusted_mode;
struct intel_connector *found = NULL, *intel_connector;
int mst_pbn;
@@ -52,11 +52,21 @@ static bool intel_dp_mst_compute_config(struct 
intel_encoder *encoder,
 * seem to suggest we should do otherwise.
 */
lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
-   intel_dp->link_bw = intel_dp_max_link_bw(intel_dp);
+
+   rate = intel_dp_max_link_rate(intel_dp);
+
+   if (intel_dp->num_supported_rates) {
+   intel_dp->link_bw = 0;
+   intel_dp->rate_select = intel_dp_rate_select(intel_dp, rate);
+   } else {
+   intel_dp->link_bw = drm_dp_link_rate_to_bw_code(rate);
+   intel_dp->rate_select = 0;
+   }
+
intel_dp->lane_count = lane_count;
 
pipe_config->pipe_bpp = 24;
-   pipe_config->port_clock = 
drm_dp_bw_code_to_link_rate(intel_dp->link_bw);
+   pipe_config->port_clock = rate;
 
for_each_intel_connector(dev, intel_connector) {
if (intel_connector->new_encoder == encoder) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d5a1f1f..4174198 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1061,8 +1061,8 @@ void intel_edp_panel_off(struct intel_dp *intel_dp);
 void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector 
*connector);
 void intel_dp_mst_suspend(struct drm_device *dev);
 void intel_dp_mst_resume(struct drm_device *dev);
-int intel_dp_max_link_bw(struct intel_dp *intel_dp);
 int intel_dp_max_link_rate(struct intel_dp *intel_dp);
+int intel_dp_rate_select(struct intel_dp *intel_dp, int rate);
 void intel_dp_hot_plug(struct intel_encoder *intel_encoder);
 void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv);
 uint32_t intel_dp_pack_aux(const uint8_t *src, int src_bytes);
-- 
2.0.5

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

[Intel-gfx] [PATCH 05/13] drm/i915: Remove special case from intel_supported_rates()

[ville . syrjala]

From: Ville Syrjälä 

Now that both source and sink rates are always filled in there's no need
for any special cases in intel_supported_rates().

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_dp.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d6098a0..aa373a2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1204,14 +1204,6 @@ static int intel_supported_rates(const int 
*source_rates, int source_len,
 {
int i = 0, j = 0, k = 0;
 
-   /* For panels with edp version less than 1.4 */
-   if (sink_len == 0) {
-   for (i = 0; i < source_len; ++i)
-   supported_rates[i] = source_rates[i];
-   return source_len;
-   }
-
-   /* For edp1.4 panels, find the common rates between source and sink */
while (i < source_len && j < sink_len) {
if (source_rates[i] == sink_rates[j]) {
supported_rates[k] = source_rates[i];
-- 
2.0.5

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

[Intel-gfx] [PATCH 02/13] drm/i915: Store the converted link rates in intel_dp->supported_rates[]

[ville . syrjala]

From: Ville Syrjälä 

No point in converting from hardware format every single time, just
store the rates in the final format under intel_dp.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_dp.c  | 33 +++--
 drivers/gpu/drm/i915/intel_drv.h |  3 ++-
 2 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1fa8cc1..d638f5e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1141,8 +1141,6 @@ static int
 intel_read_sink_rates(struct intel_dp *intel_dp, int *sink_rates)
 {
struct drm_device *dev = intel_dp_to_dev(intel_dp);
-   int i = 0;
-   uint16_t val;
 
if (INTEL_INFO(dev)->gen >= 9 && intel_dp->supported_rates[0]) {
/*
@@ -1150,18 +1148,12 @@ intel_read_sink_rates(struct intel_dp *intel_dp, int 
*sink_rates)
 * link rate table method, so read link rates from
 * supported_link_rates
 */
-   for (i = 0; i < DP_MAX_SUPPORTED_RATES; ++i) {
-   val = le16_to_cpu(intel_dp->supported_rates[i]);
-   if (val == 0)
-   break;
-
-   sink_rates[i] = val * 200;
-   }
+   memcpy(sink_rates, intel_dp->supported_rates,
+  sizeof(intel_dp->supported_rates));
 
-   if (i <= 0)
-   DRM_ERROR("No rates in SUPPORTED_LINK_RATES");
+   return intel_dp->num_supported_rates;
}
-   return i;
+   return 0;
 }
 
 static int
@@ -3751,10 +3743,23 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
(intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] & 
DP_DPCD_DISPLAY_CONTROL_CAPABLE) &&
(intel_dp_dpcd_read_wake(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) 
== 1) &&
(rev >= 0x03)) { /* eDp v1.4 or higher */
+   __le16 supported_rates[DP_MAX_SUPPORTED_RATES];
+   int i;
+
intel_dp_dpcd_read_wake(&intel_dp->aux,
DP_SUPPORTED_LINK_RATES,
-   intel_dp->supported_rates,
-   sizeof(intel_dp->supported_rates));
+   supported_rates,
+   sizeof(supported_rates));
+
+   for (i = 0; i < ARRAY_SIZE(supported_rates); i++) {
+   int val = le16_to_cpu(supported_rates[i]);
+
+   if (val == 0)
+   break;
+
+   intel_dp->supported_rates[i] = val * 200;
+   }
+   intel_dp->num_supported_rates = i;
}
if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
  DP_DWN_STRM_PORT_PRESENT))
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c77128c..69c8437 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -627,7 +627,8 @@ struct intel_dp {
uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
-   __le16 supported_rates[DP_MAX_SUPPORTED_RATES];
+   uint8_t num_supported_rates;
+   int supported_rates[DP_MAX_SUPPORTED_RATES];
struct drm_dp_aux aux;
uint8_t train_set[4];
int panel_power_up_delay;
-- 
2.0.5

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

[Intel-gfx] [PATCH 06/13] drm/i915: Fully separate source vs. sink rates

[ville . syrjala]

From: Ville Syrjälä 

Remove the sink vs. source limit mess from intel_dp_max_link_bw() and
just move the source restriction checks to intel_dp_source_rates().

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_dp.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index aa373a2..61538f4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -126,19 +126,11 @@ int
 intel_dp_max_link_bw(struct intel_dp *intel_dp)
 {
int max_link_bw = intel_dp->dpcd[DP_MAX_LINK_RATE];
-   struct drm_device *dev = intel_dp->attached_connector->base.dev;
 
switch (max_link_bw) {
case DP_LINK_BW_1_62:
case DP_LINK_BW_2_7:
-   break;
-   case DP_LINK_BW_5_4: /* 1.2 capable displays may advertise higher bw */
-   if (((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) ||
-INTEL_INFO(dev)->gen >= 8) &&
-   intel_dp->dpcd[DP_DPCD_REV] >= 0x12)
-   max_link_bw = DP_LINK_BW_5_4;
-   else
-   max_link_bw = DP_LINK_BW_2_7;
+   case DP_LINK_BW_5_4:
break;
default:
WARN(1, "invalid max DP link bw val %x, using 1.62Gbps\n",
@@ -1151,10 +1143,8 @@ intel_dp_sink_rates(struct intel_dp *intel_dp, const int 
**sink_rates)
 }
 
 static int
-intel_dp_source_rates(struct intel_dp *intel_dp, const int **source_rates)
+intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
 {
-   struct drm_device *dev = intel_dp_to_dev(intel_dp);
-
if (INTEL_INFO(dev)->gen >= 9) {
*source_rates = gen9_rates;
return ARRAY_SIZE(gen9_rates);
@@ -1162,7 +1152,11 @@ intel_dp_source_rates(struct intel_dp *intel_dp, const 
int **source_rates)
 
*source_rates = default_rates;
 
-   return (intel_dp_max_link_bw(intel_dp) >> 3) + 1;
+   if (INTEL_INFO(dev)->gen >= 8 ||
+   (IS_HASWELL(dev) && !IS_HSW_ULX(dev)))
+   return (DP_LINK_BW_5_4 >> 3) + 1;
+   else
+   return (DP_LINK_BW_2_7 >> 3) + 1;
 }
 
 static void
@@ -1256,7 +1250,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 
sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
 
-   source_len = intel_dp_source_rates(intel_dp, &source_rates);
+   source_len = intel_dp_source_rates(dev, &source_rates);
 
supported_len = intel_supported_rates(source_rates, source_len,
sink_rates, sink_len, supported_rates);
-- 
2.0.5

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

Re: [Intel-gfx] [PATCH 1/2] gpio/crystalcove: Export Panel and backlight en/disable signals as GPIO

[Kumar, Shobhit]

On Mon, 2015-03-09 at 18:15 +0100, Linus Walleij wrote:
> On Fri, Mar 6, 2015 at 5:23 PM, Kumar, Shobhit  
> wrote:
> 
> > There are actually two lines for Panel Power control and Backlight
> > enable/disable. I have already moved towards adding a new Cell device
> > for PWM child device and a new pwm driver for the same. That will take
> > care of backlight thingy. But for the Panel power control, I am at loss
> > for how best to program that.
> 
> Isn't it just a very simple regulator (just on/off of fixed voltage) cell?
> 

It is just behaving as an output GPIO line and for that reason I feel
that rather than adding a new regulator driver, it is better to augment
the gpio-crystalcove driver. I am planning to export as GPIO from PMIC
MFD driver by way of lookup table and use in display side. Will send RFC
patch soon.

Backlight control as I said is a new pwm cell.

Regards
Shobhit

> Yours,
> Linus Walleij

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

[Intel-gfx] [PATCH 00/13] drm/i915: Clean up the DP link rate code

[ville . syrjala]

From: Ville Syrjälä 

We have a bit of a mess with the source vs. sink link rate handling, so I
went ahead and tried to clean it up. So now we keep the source and sink
rates neatly in their own corners and compute the intersection when needed.

I considered storing the intersection itself under intel_dp, but for
eDP 1.4 we need to convert the chose rate back to the index from which
we got, which means keeping the original sink rates around as well. So
in the end I figured it's not a huge deal if we keep computing the
intersection on demand.

I also ended up adding eDP 1.4 intermediate frequency support for CHV, but
as I don't have an eDP 1.4 panel in my BSW I couldn't actually test it.

Ville Syrjälä (13):
  drm/i915: Make the DP rates int instead of uint32_t
  drm/i915: Store the converted link rates in
intel_dp->supported_rates[]
  drm/i915: Don't copy the DP source rates arrays
  drm/i915: Don't copy sink rates either
  drm/i915: Remove special case from intel_supported_rates()
  drm/i915: Fully separate source vs. sink rates
  drm/i915: Hide the source vs. sink rate handling from
intel_dp_compute_config()
  drm/i915: Fix max link rate in intel_dp_mode_valid()
  drm/i915: Use DP_LINK_RATE_SET whenever possible
  drm/i915: Fix MST link rate handling
  drm/i915: Avoid overflowing the DP link rate arrays
  drm/i915: Add eDP intermediate frequencies for CHV
  drm/i915: Include the sink/source/supported rates in debug output

 drivers/gpu/drm/i915/intel_dp.c | 221 +++-
 drivers/gpu/drm/i915/intel_dp_mst.c |  16 ++-
 drivers/gpu/drm/i915/intel_drv.h|   6 +-
 3 files changed, 157 insertions(+), 86 deletions(-)

-- 
2.0.5

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

[Intel-gfx] [PATCH 01/13] drm/i915: Make the DP rates int instead of uint32_t

[ville . syrjala]

From: Ville Syrjälä 

No point in using uint32_t here, just plain old int will do.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_dp.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 33d5877..1fa8cc1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -85,10 +85,9 @@ static const struct dp_link_dpll chv_dpll[] = {
{ .p1 = 2, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } }
 };
 /* Skylake supports following rates */
-static const uint32_t gen9_rates[] = { 162000, 216000, 27, 324000,
-   432000, 54 };
-
-static const uint32_t default_rates[] = { 162000, 27, 54 };
+static const int gen9_rates[] = { 162000, 216000, 27,
+ 324000, 432000, 54 };
+static const int default_rates[] = { 162000, 27, 54 };
 
 /**
  * is_edp - is the given port attached to an eDP panel (either CPU or PCH)
@@ -1139,7 +1138,7 @@ hsw_dp_set_ddi_pll_sel(struct intel_crtc_state 
*pipe_config, int link_bw)
 }
 
 static int
-intel_read_sink_rates(struct intel_dp *intel_dp, uint32_t *sink_rates)
+intel_read_sink_rates(struct intel_dp *intel_dp, int *sink_rates)
 {
struct drm_device *dev = intel_dp_to_dev(intel_dp);
int i = 0;
@@ -1166,7 +1165,7 @@ intel_read_sink_rates(struct intel_dp *intel_dp, uint32_t 
*sink_rates)
 }
 
 static int
-intel_read_source_rates(struct intel_dp *intel_dp, uint32_t *source_rates)
+intel_read_source_rates(struct intel_dp *intel_dp, int *source_rates)
 {
struct drm_device *dev = intel_dp_to_dev(intel_dp);
int i;
@@ -1217,8 +1216,9 @@ intel_dp_set_clock(struct intel_encoder *encoder,
}
 }
 
-static int intel_supported_rates(const uint32_t *source_rates, int source_len,
-const uint32_t *sink_rates, int sink_len, uint32_t *supported_rates)
+static int intel_supported_rates(const int *source_rates, int source_len,
+const int *sink_rates, int sink_len,
+int *supported_rates)
 {
int i = 0, j = 0, k = 0;
 
@@ -1245,7 +1245,7 @@ const uint32_t *sink_rates, int sink_len, uint32_t 
*supported_rates)
return k;
 }
 
-static int rate_to_index(uint32_t find, const uint32_t *rates)
+static int rate_to_index(int find, const int *rates)
 {
int i = 0;
 
@@ -1275,9 +1275,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
int max_clock;
int bpp, mode_rate;
int link_avail, link_clock;
-   uint32_t sink_rates[8];
-   uint32_t supported_rates[8] = {0};
-   uint32_t source_rates[8];
+   int sink_rates[8];
+   int supported_rates[8] = {0};
+   int source_rates[8];
int source_len, sink_len, supported_len;
 
sink_len = intel_read_sink_rates(intel_dp, sink_rates);
-- 
2.0.5

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

Re: [Intel-gfx] [PATCH v3] drm/i915: Optimistically spin for the request completion

[Tvrtko Ursulin]

On 03/12/2015 01:18 PM, Chris Wilson wrote:

On Thu, Mar 12, 2015 at 01:14:30PM +, Tvrtko Ursulin wrote:

On 03/12/2015 11:11 AM, Chris Wilson wrote:

This provides a nice boost to mesa in swap bound scenarios (as mesa
throttles itself to the previous frame and given the scenario that will
complete shortly). It will also provide a good boost to systems running
with semaphores disabled and so frequently waiting on the GPU as it
switches rings. In the most favourable of microbenchmarks, this can
increase performance by around 15% - though in practice improvements
will be marginal and rarely noticeable.

v2: Account for user timeouts
v3: Limit the spinning to a single jiffie (~1us) at most. On an
otherwise idle system, there is no scheduler contention and so without a
limit we would spin until the GPU is ready.


Isn't one jiffie 1-10ms typically?


1ms. I was just thinking of doing USECS_PER_SEC / HZ, then realised that
was a jiffie, hence the confusion. At any rate, it is still the minimum
we can trivially wait for (without an expensive hrtimer).


Unless I lost track with the times, that's CONFIG_HZ right?

I don't know what server distributions do, but this Ubuntu LTS I am 
running has HZ=250 which means 4ms.


That would mean on a system where throughput is more important than 
latency, you lose most throughput by spinning the longest. In theory at 
least, no?


So perhaps which should be a tunable? Optionally auto-select the initial 
state based on HZ.


Regards,

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

[Intel-gfx] [PATCH 08/13] drm/i915: Fix max link rate in intel_dp_mode_valid()

[ville . syrjala]

From: Ville Syrjälä 

Consider the link rates reported by the sink via
DP_SUPPORTED_LINK_RATES when checking modes against the max link
rate.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_dp.c  | 15 ++-
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a88f932..63c3ef4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -206,7 +206,7 @@ intel_dp_mode_valid(struct drm_connector *connector,
target_clock = fixed_mode->clock;
}
 
-   max_link_clock = 
drm_dp_bw_code_to_link_rate(intel_dp_max_link_bw(intel_dp));
+   max_link_clock = intel_dp_max_link_rate(intel_dp);
max_lanes = intel_dp_max_lane_count(intel_dp);
 
max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
@@ -1239,6 +1239,19 @@ static int rate_to_index(int find, const int *rates)
return i;
 }
 
+int
+intel_dp_max_link_rate(struct intel_dp *intel_dp)
+{
+   int rates[DP_MAX_SUPPORTED_RATES] = {};
+   int len;
+
+   len = intel_supported_rates(intel_dp, rates);
+   if (WARN_ON(len <= 0))
+   return 162000;
+
+   return rates[rate_to_index(0, rates) - 1];
+}
+
 bool
 intel_dp_compute_config(struct intel_encoder *encoder,
struct intel_crtc_state *pipe_config)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 69c8437..d5a1f1f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1062,6 +1062,7 @@ void intel_dp_add_properties(struct intel_dp *intel_dp, 
struct drm_connector *co
 void intel_dp_mst_suspend(struct drm_device *dev);
 void intel_dp_mst_resume(struct drm_device *dev);
 int intel_dp_max_link_bw(struct intel_dp *intel_dp);
+int intel_dp_max_link_rate(struct intel_dp *intel_dp);
 void intel_dp_hot_plug(struct intel_encoder *intel_encoder);
 void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv);
 uint32_t intel_dp_pack_aux(const uint8_t *src, int src_bytes);
-- 
2.0.5

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

[Intel-gfx] [PATCH 13/13] drm/i915: Include the sink/source/supported rates in debug output

[ville . syrjala]

From: Ville Syrjälä 

TODO: Is there an actually nice way to print an array of ints?

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_dp.c | 43 +
 1 file changed, 43 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8392bd3..715694a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1236,6 +1236,46 @@ static int intel_supported_rates(struct intel_dp 
*intel_dp,
   supported_rates);
 }
 
+static void snprintf_int_array(char *str, size_t len,
+  const int *array, int nelem)
+{
+   int i;
+
+   str[0] = '\0';
+
+   for (i = 0; i < nelem; i++) {
+   int r = snprintf(str, len, "%d,", array[i]);
+   if (r >= len)
+   return;
+   str += r;
+   len -= r;
+   }
+}
+
+static void intel_dp_print_rates(struct intel_dp *intel_dp)
+{
+   struct drm_device *dev = intel_dp_to_dev(intel_dp);
+   const int *source_rates, *sink_rates;
+   int source_len, sink_len, supported_len;
+   int supported_rates[DP_MAX_SUPPORTED_RATES];
+   char str[128]; /* FIXME: too big for stack? */
+
+   if ((drm_debug & DRM_UT_KMS) == 0)
+   return;
+
+   source_len = intel_dp_source_rates(dev, &source_rates);
+   snprintf_int_array(str, sizeof(str), source_rates, source_len);
+   DRM_DEBUG_KMS("source rates: %s\n", str);
+
+   sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
+   snprintf_int_array(str, sizeof(str), sink_rates, sink_len);
+   DRM_DEBUG_KMS("sink rates: %s\n", str);
+
+   supported_len = intel_supported_rates(intel_dp, supported_rates);
+   snprintf_int_array(str, sizeof(str), supported_rates, supported_len);
+   DRM_DEBUG_KMS("supported rates: %s\n", str);
+}
+
 static int rate_to_index(int find, const int *rates)
 {
int i = 0;
@@ -3772,6 +3812,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
}
intel_dp->num_supported_rates = i;
}
+
+   intel_dp_print_rates(intel_dp);
+
if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
  DP_DWN_STRM_PORT_PRESENT))
return true; /* native DP sink */
-- 
2.0.5

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

[Intel-gfx] [PATCH 09/13] drm/i915: Use DP_LINK_RATE_SET whenever possible

[ville . syrjala]

From: Ville Syrjälä 

Drop the gen9 checks from the code and issue DP_LINK_RATE_SET whenever
the sink reports to support it.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_dp.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 63c3ef4..28d9249 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1368,14 +1368,15 @@ found:
 
intel_dp->lane_count = lane_count;
 
-   intel_dp->link_bw =
-   drm_dp_link_rate_to_bw_code(supported_rates[clock]);
-
-   if (INTEL_INFO(dev)->gen >= 9 && intel_dp->supported_rates[0]) {
+   if (intel_dp->num_supported_rates) {
+   intel_dp->link_bw = 0;
intel_dp->rate_select =
rate_to_index(supported_rates[clock],
  intel_dp->supported_rates);
-   intel_dp->link_bw = 0;
+   } else {
+   intel_dp->link_bw =
+   drm_dp_link_rate_to_bw_code(supported_rates[clock]);
+   intel_dp->rate_select = 0;
}
 
pipe_config->pipe_bpp = bpp;
@@ -3489,7 +3490,7 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config, 2);
-   if (INTEL_INFO(dev)->gen >= 9 && intel_dp->supported_rates[0])
+   if (intel_dp->num_supported_rates)
drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_RATE_SET,
&intel_dp->rate_select, 1);
 
-- 
2.0.5

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

[Intel-gfx] [PATCH 12/13] drm/i915: Add eDP intermediate frequencies for CHV

[ville . syrjala]

From: Ville Syrjälä 

"P1273_DPLL_Programming Spreadsheet.xlsm" lists a boatload of
frequencies for eDP. Try to use them all.

For now I've decided not to add hardcoded DPLL dividers for these cases
since chv_find_best_dpll() works just fine.

I've not actually tested any of these since I don't have an eDP 1.4 panel.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_dp.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a088186..8392bd3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -87,6 +87,9 @@ static const struct dp_link_dpll chv_dpll[] = {
 /* Skylake supports following rates */
 static const int gen9_rates[] = { 162000, 216000, 27,
  324000, 432000, 54 };
+static const int chv_rates[] = { 162000, 202500, 21, 216000,
+243000, 27, 324000, 405000,
+42, 432000, 54 };
 static const int default_rates[] = { 162000, 27, 54 };
 
 /**
@@ -1148,6 +1151,9 @@ intel_dp_source_rates(struct drm_device *dev, const int 
**source_rates)
if (INTEL_INFO(dev)->gen >= 9) {
*source_rates = gen9_rates;
return ARRAY_SIZE(gen9_rates);
+   } else if (IS_CHERRYVIEW(dev)) {
+   *source_rates = chv_rates;
+   return ARRAY_SIZE(chv_rates);
}
 
*source_rates = default_rates;
-- 
2.0.5

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

[Intel-gfx] [PATCH 04/13] drm/i915: Don't copy sink rates either

[ville . syrjala]

From: Ville Syrjälä 

Once we've read the rates from the sink we don't have to mess with them,
so the caller can just look at the stored rates without doing extra
copies. If the sink doesn't support the new link rate stuff, we just
point the caller at the default_rates[] array.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_dp.c | 24 +---
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 537f1d0..d6098a0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1138,22 +1138,16 @@ hsw_dp_set_ddi_pll_sel(struct intel_crtc_state 
*pipe_config, int link_bw)
 }
 
 static int
-intel_read_sink_rates(struct intel_dp *intel_dp, int *sink_rates)
+intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates)
 {
-   struct drm_device *dev = intel_dp_to_dev(intel_dp);
-
-   if (INTEL_INFO(dev)->gen >= 9 && intel_dp->supported_rates[0]) {
-   /*
-* Receiver supports only main-link rate selection by
-* link rate table method, so read link rates from
-* supported_link_rates
-*/
-   memcpy(sink_rates, intel_dp->supported_rates,
-  sizeof(intel_dp->supported_rates));
-
+   if (intel_dp->num_supported_rates) {
+   *sink_rates = intel_dp->supported_rates;
return intel_dp->num_supported_rates;
}
-   return 0;
+
+   *sink_rates = default_rates;
+
+   return (intel_dp_max_link_bw(intel_dp) >> 3) + 1;
 }
 
 static int
@@ -1263,12 +1257,12 @@ intel_dp_compute_config(struct intel_encoder *encoder,
int max_clock;
int bpp, mode_rate;
int link_avail, link_clock;
-   int sink_rates[8];
+   const int *sink_rates;
int supported_rates[8] = {0};
const int *source_rates;
int source_len, sink_len, supported_len;
 
-   sink_len = intel_read_sink_rates(intel_dp, sink_rates);
+   sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
 
source_len = intel_dp_source_rates(intel_dp, &source_rates);
 
-- 
2.0.5

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

Re: [Intel-gfx] [PATCH v2] drm/i915: Fix BUG in i915_gem.c when switch to console

[Matt Roper]

On Thu, Mar 12, 2015 at 10:28:56AM +0200, Jani Nikula wrote:
> On Thu, 12 Mar 2015, Xi Ruoyao  wrote:
> > In intel_crtc_page_flip, intel_display.c, the code changed the framebuffer
> > assigned to plane crtc->primary by
> >
> > crtc->primary->fb = fb;
> >
> > However, it forgot to change crtc->primary->state->fb. However, when we
> > switch to console, some kernel code will read crtc->primary->state->fb
> > to get the framebuffer assigned to crtc->primaty. Then a framebuffer
> > object can be unpinned twice and a kernel BUG will be produced in 
> > i915_gem.c.
> >
> > So, update crtc->primary->state->fb in intel_display.c using
> > drm_atomic_set_fb_for_plane to fix the BUG.
> >
> > Signed-off-by: Xi Ruoyao 
> > Fixed: Bug 93711 
> 
> Is this a problem with drm-intel-nightly? In particular see
> 
> commit afd65eb4cc0578a9c07d621acdb8a570e2782bf7
> Author: Matt Roper 
> Date:   Tue Feb 3 13:10:04 2015 -0800
> 
> drm/i915: Ensure plane->state->fb stays in sync with plane->fb
> 
> Matt, do you think this fixes the described issue? Can we backport to
> drm-intel-fixes (and v4.0)?
> 
> BR,
> Jani.
> 

Yeah, Xi's patch should be the equivalent of my even earlier patch:

commit db068420560511de80ac59222644f2bdf278c3d5
Author: Matt Roper 
Date:   Fri Jan 30 16:22:36 2015 -0800

drm/i915: Keep plane->state updated on pageflip

so backporting either of those would be fine to solve the issue.  The
One you reference above also cleans up more places where we get out of
sync, but I'm not sure if any of those cause immediate problems; I think
those were just preparation for other -next work that was on the way.


Matt

> 
> 
> > ---
> >  Sorry, the previous patch is mangled by email client, so I am re-sending 
> > it.
> >
> >  drivers/gpu/drm/i915/intel_display.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index e730789..97083fd 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -37,6 +37,7 @@
> >  #include 
> >  #include "i915_drv.h"
> >  #include "i915_trace.h"
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -9816,6 +9817,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> > drm_gem_object_reference(&obj->base);
> >  
> > crtc->primary->fb = fb;
> > +   drm_atomic_set_fb_for_plane(crtc->primary->state, fb);
> >  
> > work->pending_flip_obj = obj;
> >  
> > -- 
> > 1.9.1
> >
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t] lib: move igt_interactive_debug into igt_core.c

[Thomas Wood]

igt_interactive_debug should be defined in igt_core.c, rather than the
header, to avoid it being defined more than once.

Reported-by: Damien Lespiau 
Signed-off-by: Thomas Wood 
---
 lib/igt_core.c | 1 +
 lib/igt_core.h | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index c217a01..4ae3524 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -209,6 +209,7 @@
  */
 
 static unsigned int exit_handler_count;
+const char *igt_interactive_debug;
 
 /* subtests helpers */
 static bool list_subtests = false;
diff --git a/lib/igt_core.h b/lib/igt_core.h
index c2c820d..0716000 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -557,7 +557,7 @@ bool igt_run_in_simulation(void);
 
 void igt_skip_on_simulation(void);
 
-const char *igt_interactive_debug;
+extern const char *igt_interactive_debug;
 
 /* structured logging */
 enum igt_log_level {
-- 
2.1.0

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

Re: [Intel-gfx] [PATCH] drm/i915: Ensure plane->state->fb stays in sync with plane->fb

[Matt Roper]

On Thu, Mar 12, 2015 at 02:45:31PM +0200, Jani Nikula wrote:
> 
> Matt, please review or suggest an alternative for v4.0-rc.
> 
> Thanks,
> Jani.

Yep, this looks good to me.  So

Reviewed-by: Matt Roper 


Matt

> 
> 
> On Thu, 12 Mar 2015, Xi Ruoyao  wrote:
> > plane->state->fb and plane->fb should always reference the same FB so
> > that atomic and legacy codepaths have the same view of display state.
> > However, there are some places in kernel code that directly set
> > plane->fb and neglect to update plane->state->fb. If we never do a
> > successful update through the atomic pipeline, the RmFB cleanup code
> > will look at the plane->state->fb pointer, which has never actually
> > been set to a legitimate value, and try to clean it up, leading to
> > BUG's.
> >
> > Add a quick helper function to synchronize plane->state->fb with
> > plane->fb and call it everywhere the driver tries to manually set
> > plane->fb outside of the atomic pipeline. In this function, use
> > drm_atomic_set_fb_for_plane instead of writing plane->state->fb
> > directly to keep the reference count right.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88909
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=93711
> > Signed-off-by: Matt Roper 
> > Signed-off-by: Daniel Vetter 
> > Signed-off-by: Xi Ruoyao 
> > ---
> >  This is modified from Matt Roper's patch to drm-intel-nightly with 
> >  commit id
> >
> > afd65eb4cc0578a9c07d621acdb8a570e2782bf7
> >
> >  However this bug exists in mainline kernel too, so I created this to
> >  fix it in mainline kernel.
> >
> >  A minor change is to use drm_atomic_set_fb_for_plane instead of
> >  update reference count manually.
> >
> >  drivers/gpu/drm/i915/intel_display.c | 17 +
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index e730789..c483f2f 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -37,6 +37,7 @@
> >  #include 
> >  #include "i915_drv.h"
> >  #include "i915_trace.h"
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -2416,6 +2417,14 @@ out_unref_obj:
> > return false;
> >  }
> >  
> > +/* Update plane->state->fb to match plane->fb after driver-internal 
> > updates */
> > +static void
> > +update_state_fb(struct drm_plane *plane)
> > +{
> > +   if (plane->fb != plane->state->fb)
> > +   drm_atomic_set_fb_for_plane(plane->state, plane->fb);
> > +}
> > +
> >  static void
> >  intel_find_plane_obj(struct intel_crtc *intel_crtc,
> >  struct intel_initial_plane_config *plane_config)
> > @@ -2462,6 +2471,8 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc,
> > break;
> > }
> > }
> > +
> > +   update_state_fb(intel_crtc->base.primary);
> >  }
> >  
> >  static void i9xx_update_primary_plane(struct drm_crtc *crtc,
> > @@ -6650,6 +6661,7 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
> >   plane_config->size);
> >  
> > crtc->base.primary->fb = fb;
> > +   update_state_fb(crtc->base.primary);
> >  }
> >  
> >  static void chv_crtc_clock_get(struct intel_crtc *crtc,
> > @@ -7687,6 +7699,7 @@ skylake_get_initial_plane_config(struct intel_crtc 
> > *crtc,
> >   plane_config->size);
> >  
> > crtc->base.primary->fb = fb;
> > +   update_state_fb(crtc->base.primary);
> > return;
> >  
> >  error:
> > @@ -7778,6 +7791,7 @@ ironlake_get_initial_plane_config(struct intel_crtc 
> > *crtc,
> >   plane_config->size);
> >  
> > crtc->base.primary->fb = fb;
> > +   update_state_fb(crtc->base.primary);
> >  }
> >  
> >  static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> > @@ -9816,6 +9830,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> > drm_gem_object_reference(&obj->base);
> >  
> > crtc->primary->fb = fb;
> > +   update_state_fb(crtc->primary);
> >  
> > work->pending_flip_obj = obj;
> >  
> > @@ -9884,6 +9899,7 @@ cleanup_unpin:
> >  cleanup_pending:
> > atomic_dec(&intel_crtc->unpin_work_count);
> > crtc->primary->fb = old_fb;
> > +   update_state_fb(crtc->primary);
> > drm_gem_object_unreference(&work->old_fb_obj->base);
> > drm_gem_object_unreference(&obj->base);
> > mutex_unlock(&dev->struct_mutex);
> > @@ -13718,6 +13734,7 @@ void intel_modeset_gem_init(struct drm_device *dev)
> >   to_intel_crtc(c)->pipe);
> > drm_framebuffer_unreference(c->primary->fb);
> > c->primary->fb = NULL;
> > +   update_state_fb(c->primary);
> > }
> > }
> > mutex_unlock(&dev->struct_mutex);
> > -- 
> > 1.9.1
> >
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_

Re: [Intel-gfx] [PATCH v3] drm/i915: Optimistically spin for the request completion

[Chris Wilson]

On Thu, Mar 12, 2015 at 03:18:01PM +, Tvrtko Ursulin wrote:
> On 03/12/2015 01:18 PM, Chris Wilson wrote:
> >1ms. I was just thinking of doing USECS_PER_SEC / HZ, then realised that
> >was a jiffie, hence the confusion. At any rate, it is still the minimum
> >we can trivially wait for (without an expensive hrtimer).
> 
> Unless I lost track with the times, that's CONFIG_HZ right?
> 
> I don't know what server distributions do, but this Ubuntu LTS I am
> running has HZ=250 which means 4ms.
> 
> That would mean on a system where throughput is more important than
> latency, you lose most throughput by spinning the longest. In theory
> at least, no?

Only in theory, and only if you mean throughput of non-i915 workloads
with preemption disabled.  Spinning here improves both latency and
throughput for gfx clients. Using up the timeslice for the client may
have secondary effects though - otherwise they would get iowait credits.

> So perhaps which should be a tunable? Optionally auto-select the
> initial state based on HZ.

Spinning less than a jiffie requires hrtimers at which point you may as
well just use the i915 interrupt (rather than setup a timer interrupt).
-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

[Intel-gfx] [RFC v5 8/9] drivers/mfd: ADD PWM lookup table for CRC PMIC based PWM

[Shobhit Kumar]

On some BYT PLatform the PWM is controlled using CRC PMIC. Add a lookup
entry for the same to be used by the consumer (Intel GFX)

CC: Samuel Ortiz 
Cc: Linus Walleij 
Cc: Alexandre Courbot 
Cc: Thierry Reding 
Signed-off-by: Shobhit Kumar 
---
 drivers/mfd/intel_soc_pmic_core.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/mfd/intel_soc_pmic_core.c 
b/drivers/mfd/intel_soc_pmic_core.c
index 365d5de..b140e3c 100644
--- a/drivers/mfd/intel_soc_pmic_core.c
+++ b/drivers/mfd/intel_soc_pmic_core.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "intel_soc_pmic_core.h"
 
 /* Lookup table for the Panel Enable/Disable line as GPIO signals */
@@ -37,6 +38,11 @@ struct gpiod_lookup_table panel_gpio_table = {
},
 };
 
+/* PWM consumed by the Intel GFX */
+static struct pwm_lookup crc_pwm_lookup[] = {
+   PWM_LOOKUP("crystal_cove_pwm", 0, ":00:02.0", "pwm_backlight", 0, 
PWM_POLARITY_NORMAL),
+};
+
 /*
  * On some boards the PMIC interrupt may come from a GPIO line.
  * Try to lookup the ACPI table and see if such connection exists. If not,
@@ -99,6 +105,9 @@ static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c,
/* Add lookup table binding for Panel Control to the GPIO Chip */
gpiod_add_lookup_table(&panel_gpio_table);
 
+   /* Add lookup table for crc-pwm */
+   pwm_add_table(crc_pwm_lookup, ARRAY_SIZE(crc_pwm_lookup));
+
ret = mfd_add_devices(dev, -1, config->cell_dev,
  config->n_cell_devs, NULL, 0,
  regmap_irq_get_domain(pmic->irq_chip_data));
-- 
2.1.0

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

[Intel-gfx] [RFC v5 5/9] drivers/mfd: Add PWM cell device for Crystalcove PMIC

[Shobhit Kumar]

Needed for PWM control suuported by the PMIC

CC: Samuel Ortiz 
Cc: Linus Walleij 
Cc: Alexandre Courbot 
Cc: Thierry Reding 
Signed-off-by: Shobhit Kumar 
---
 drivers/mfd/intel_soc_pmic_crc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mfd/intel_soc_pmic_crc.c b/drivers/mfd/intel_soc_pmic_crc.c
index 4cc1b32..8839e25 100644
--- a/drivers/mfd/intel_soc_pmic_crc.c
+++ b/drivers/mfd/intel_soc_pmic_crc.c
@@ -109,6 +109,9 @@ static struct mfd_cell crystal_cove_dev[] = {
{
.name = "crystal_cove_pmic",
},
+   {
+   .name = "crystal_cove_pwm",
+   },
 };
 
 static const struct regmap_config crystal_cove_regmap_config = {
-- 
2.1.0

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

[Intel-gfx] [RFC v5 6/9] drivers/pwm: Add Crystalcove (CRC) PWM driver

[Shobhit Kumar]

The Crystalcove PMIC controls PWM signals and this driver exports that
capability as a PWM chip driver. This is platform device implementtaion
of the drivers/mfd cell device for CRC PMIC

CC: Samuel Ortiz 
Cc: Linus Walleij 
Cc: Alexandre Courbot 
Cc: Thierry Reding 
Signed-off-by: Shobhit Kumar 
---
 drivers/pwm/Kconfig   |   7 +++
 drivers/pwm/Makefile  |   1 +
 drivers/pwm/pwm-crc.c | 161 ++
 3 files changed, 169 insertions(+)
 create mode 100644 drivers/pwm/pwm-crc.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index b1541f4..954da3e 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -183,6 +183,13 @@ config PWM_LPC32XX
  To compile this driver as a module, choose M here: the module
  will be called pwm-lpc32xx.
 
+config PWM_CRC
+   bool "Intel Crystalcove (CRC) PWM support"
+   depends on X86 && INTEL_SOC_PMIC
+   help
+ Generic PWM framework driver for Crystalcove (CRC) PMIC based PWM
+ control.
+
 config PWM_LPSS
tristate "Intel LPSS PWM support"
depends on X86
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index ec50eb5..3d38fed 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -35,3 +35,4 @@ obj-$(CONFIG_PWM_TIPWMSS) += pwm-tipwmss.o
 obj-$(CONFIG_PWM_TWL)  += pwm-twl.o
 obj-$(CONFIG_PWM_TWL_LED)  += pwm-twl-led.o
 obj-$(CONFIG_PWM_VT8500)   += pwm-vt8500.o
+obj-$(CONFIG_PWM_CRC)  += pwm-crc.o
diff --git a/drivers/pwm/pwm-crc.c b/drivers/pwm/pwm-crc.c
new file mode 100644
index 000..9dc169e
--- /dev/null
+++ b/drivers/pwm/pwm-crc.c
@@ -0,0 +1,161 @@
+/*
+ * pwm-crc.c - Intel Crystal Cove PWM Driver
+ *
+ * Copyright (C) 2015 Intel Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Author: Shobhit Kumar 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define PWM0_CLK_DIV   0x4B
+#define  PWM_OUTPUT_ENABLE (1<<7)
+#define  PWM_DIV_CLK_0 0x00 /* DIVIDECLK = BASECLK */
+#define  PWM_DIV_CLK_100   0x63 /* DIVIDECLK = BASECLK/100 */
+#define  PWM_DIV_CLK_128   0x7F /* DIVIDECLK = BASECLK/128 */
+
+#define PWM0_DUTY_CYCLE0x4E
+#define BACKLIGHT_EN   0x51
+
+#define PWM_MAX_LEVEL  0xFF
+
+/**
+ * struct crystalcove_pwm - Crystal Cove PWM controller
+ * @chip: the abstract pwm_chip structure.
+ * @regmap: the regmap from the parent device.
+ */
+struct crystalcove_pwm {
+   struct pwm_chip chip;
+   struct platform_device *pdev;
+   struct regmap *regmap;
+};
+
+static inline struct crystalcove_pwm *to_crc_pwm(struct pwm_chip *pc)
+{
+   return container_of(pc, struct crystalcove_pwm, chip);
+}
+
+static int crc_pwm_config(struct pwm_chip *c, struct pwm_device *pwm,
+ int clk_div, int duty_percent)
+{
+   struct crystalcove_pwm *crc_pwm = to_crc_pwm(c);
+   struct device *dev = &crc_pwm->pdev->dev;
+   int level;
+
+   dev_dbg(dev, "crc-pwm-config\n");
+
+   if (pwm->clk_div != clk_div) {
+   /* configure the clock divider */
+   if (clk_div > PWM_DIV_CLK_128)
+   clk_div = PWM_DIV_CLK_128;
+
+   regmap_write(crc_pwm->regmap, PWM0_CLK_DIV,
+   clk_div | PWM_OUTPUT_ENABLE);
+
+   }
+
+   /* change the pwm duty cycle */
+   level = (duty_percent * PWM_MAX_LEVEL) / 100;
+   regmap_write(crc_pwm->regmap, PWM0_DUTY_CYCLE, level);
+
+   return 0;
+}
+
+static int crc_pwm_enable(struct pwm_chip *c, struct pwm_device *pwm)
+{
+   struct crystalcove_pwm *crc_pwm = to_crc_pwm(c);
+   struct device *dev = &crc_pwm->pdev->dev;
+
+   dev_dbg(dev, "crc-pwm-enable\n");
+
+   regmap_write(crc_pwm->regmap, BACKLIGHT_EN, 1);
+
+   return 0;
+}
+
+static void crc_pwm_disable(struct pwm_chip *c, struct pwm_device *pwm)
+{
+   struct crystalcove_pwm *crc_pwm = to_crc_pwm(c);
+   struct device *dev = &crc_pwm->pdev->dev;
+
+   dev_dbg(dev, "crc-pwm-disable\n");
+
+   regmap_write(crc_pwm->regmap, BACKLIGHT_EN, 0);
+}
+
+static const struct pwm_ops crc_pwm_ops = {
+   .config_alternate = crc_pwm_config,
+   .enable = crc_pwm_enable,
+   .disable = crc_pwm_disable,
+   .owner = THIS_MODULE,
+};
+
+static int crystalcove_pwm_probe(struct platform_device *pdev)
+{
+   struct crystalcove_pwm *pwm;
+   int retval;
+   struct device *dev = pdev->dev.parent;
+   struct intel_soc_pmic *pmic = dev_get_dr

[Intel-gfx] [RFC v5 3/9] drm/i915: Use the CRC gpio for panel enable/disable

[Shobhit Kumar]

The CRC (Crystal Cove) PMIC, controls the panel enable and disable
signals for BYT for dsi panels. This is indicated in the VBT fields. Use
that to initialize and use GPIO based control for these signals.

v2: Use the newer gpiod interface(Alexandre)

CC: Samuel Ortiz 
Cc: Linus Walleij 
Cc: Alexandre Courbot 
Cc: Thierry Reding 
Signed-off-by: Shobhit Kumar 
---
 drivers/gpu/drm/i915/intel_dsi.c | 34 --
 drivers/gpu/drm/i915/intel_dsi.h | 10 ++
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index c8c8b24..219421c 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "i915_drv.h"
 #include "intel_drv.h"
 #include "intel_dsi.h"
@@ -415,6 +416,13 @@ static void intel_dsi_pre_enable(struct intel_encoder 
*encoder)
 
DRM_DEBUG_KMS("\n");
 
+   /* Panel Enable over CRC PMIC */
+   if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC &&
+   intel_dsi->gpio_panel)
+   gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1);
+
+   msleep(intel_dsi->panel_on_delay);
+
/* Disable DPOunit clock gating, can stall pipe
 * and we need DPLL REFA always enabled */
tmp = I915_READ(DPLL(pipe));
@@ -432,8 +440,6 @@ static void intel_dsi_pre_enable(struct intel_encoder 
*encoder)
/* put device in ready state */
intel_dsi_device_ready(encoder);
 
-   msleep(intel_dsi->panel_on_delay);
-
drm_panel_prepare(intel_dsi->panel);
 
for_each_dsi_port(port, intel_dsi->ports)
@@ -576,6 +582,11 @@ static void intel_dsi_post_disable(struct intel_encoder 
*encoder)
 
msleep(intel_dsi->panel_off_delay);
msleep(intel_dsi->panel_pwr_cycle_delay);
+
+   /* Panel Disable over CRC PMIC */
+   if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC &&
+intel_dsi->gpio_panel)
+   gpiod_set_value_cansleep(intel_dsi->gpio_panel, 0);
 }
 
 static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
@@ -955,6 +966,11 @@ static void intel_dsi_encoder_destroy(struct drm_encoder 
*encoder)
/* XXX: Logically this call belongs in the panel driver. */
drm_panel_remove(intel_dsi->panel);
}
+
+   /* dispose of the gpios */
+   if (intel_dsi->gpio_panel)
+   gpiod_put(intel_dsi->gpio_panel);
+
intel_encoder_destroy(encoder);
 }
 
@@ -1070,6 +1086,20 @@ void intel_dsi_init(struct drm_device *dev)
goto err;
}
 
+   /*
+* In case of BYT with CRC PMIC, we need to use GPIO for
+* Panel control.
+*/
+   if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC) {
+   intel_dsi->gpio_panel =
+   gpiod_get(dev->dev, "panel", GPIOD_OUT_HIGH);
+
+   if (IS_ERR(intel_dsi->gpio_panel)) {
+   DRM_ERROR("Failed to own gpio for panel control\n");
+   intel_dsi->gpio_panel = NULL;
+   }
+   }
+
intel_encoder->type = INTEL_OUTPUT_DSI;
intel_encoder->cloneable = 0;
drm_connector_init(dev, connector, &intel_dsi_connector_funcs,
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 2784ac4..8be75a7 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -29,6 +29,13 @@
 #include 
 #include "intel_drv.h"
 
+/* CRC PMIC GPIO Access */
+#define GPIO_CHIP_NAME "gpio_crystalcove"
+#define GPIO_PANEL_EN  94
+
+#define PPS_BLC_PMIC   0
+#define PPS_BLC_SOC1
+
 /* Dual Link support */
 #define DSI_DUAL_LINK_NONE 0
 #define DSI_DUAL_LINK_FRONT_BACK   1
@@ -42,6 +49,9 @@ struct intel_dsi {
struct drm_panel *panel;
struct intel_dsi_host *dsi_hosts[I915_MAX_PORTS];
 
+   /* GPIO Desc for CRC based Panel control */
+   struct gpio_desc *gpio_panel;
+
struct intel_connector *attached_connector;
 
/* bit mask of ports being driven */
-- 
2.1.0

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

[Intel-gfx] [RFC v5 7/9] drivers/pwm: Remove __init initializer for pwm_add_table

[Shobhit Kumar]

For platforms where there are DT, some early MFD modules can reguster
lookup tables. Remove __init initializer so that this works. This is
similar to gpio_add_lookup_table which allows later initializations

CC: Samuel Ortiz 
Cc: Linus Walleij 
Cc: Alexandre Courbot 
Cc: Thierry Reding 
Signed-off-by: Shobhit Kumar 
---
 drivers/pwm/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index d979bc0..485c75e 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -597,7 +597,7 @@ EXPORT_SYMBOL_GPL(of_pwm_get);
  * @table: array of consumers to register
  * @num: number of consumers in table
  */
-void __init pwm_add_table(struct pwm_lookup *table, size_t num)
+void pwm_add_table(struct pwm_lookup *table, size_t num)
 {
mutex_lock(&pwm_lookup_lock);
 
-- 
2.1.0

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

[Intel-gfx] [RFC v5 1/9] drivers/mfd: Add lookup table for Panel Control as GPIO signal

[Shobhit Kumar]

On some Intel SoC platforms, the panel enable/disable signals are
controlled by CRC PMIC. Add those control as a new GPIO in a lookup
table for gpio-crystalcove chip during CRC driver load

CC: Samuel Ortiz 
Cc: Linus Walleij 
Cc: Alexandre Courbot 
Cc: Thierry Reding 
Signed-off-by: Shobhit Kumar 
---
 drivers/mfd/intel_soc_pmic_core.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/mfd/intel_soc_pmic_core.c 
b/drivers/mfd/intel_soc_pmic_core.c
index 80cef04..365d5de 100644
--- a/drivers/mfd/intel_soc_pmic_core.c
+++ b/drivers/mfd/intel_soc_pmic_core.c
@@ -24,8 +24,19 @@
 #include 
 #include 
 #include 
+#include 
 #include "intel_soc_pmic_core.h"
 
+/* Lookup table for the Panel Enable/Disable line as GPIO signals */
+struct gpiod_lookup_table panel_gpio_table = {
+   /* Intel GFX is consumer */
+   .dev_id = ":00:02.0",
+   .table = {
+   /* Panel EN/DISABLE */
+   GPIO_LOOKUP("gpio_crystalcove", 94, "panel", GPIO_ACTIVE_HIGH),
+   },
+};
+
 /*
  * On some boards the PMIC interrupt may come from a GPIO line.
  * Try to lookup the ACPI table and see if such connection exists. If not,
@@ -85,6 +96,9 @@ static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c,
if (ret)
dev_warn(dev, "Can't enable IRQ as wake source: %d\n", ret);
 
+   /* Add lookup table binding for Panel Control to the GPIO Chip */
+   gpiod_add_lookup_table(&panel_gpio_table);
+
ret = mfd_add_devices(dev, -1, config->cell_dev,
  config->n_cell_devs, NULL, 0,
  regmap_irq_get_domain(pmic->irq_chip_data));
-- 
2.1.0

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

[Intel-gfx] [RFC v5 4/9] drivers/pwm: Add helper to configure pwm using clock divisor and duty percent

[Shobhit Kumar]

Some chips instead of using period_ns and duty_ns can be configured
using the clock divisor and duty percent. Adds an alternative
configuration method for such chips

CC: Samuel Ortiz 
Cc: Linus Walleij 
Cc: Alexandre Courbot 
Cc: Thierry Reding 
Signed-off-by: Shobhit Kumar 
---
 drivers/pwm/core.c  | 24 
 include/linux/pwm.h | 33 +
 2 files changed, 57 insertions(+)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 810aef3..d979bc0 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -422,6 +422,30 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int 
period_ns)
 EXPORT_SYMBOL_GPL(pwm_config);
 
 /**
+ * pwm_config_alternate() - change a PWM device configuration
+ * @pwm: PWM device
+ * @clk_div: Clock divider to configure
+ * @duty_percentage: duty cycle in percentage
+ */
+int pwm_config_alternate(struct pwm_device *pwm, int clk_div, int duty_percent)
+{
+   int err;
+
+   if (!pwm || clk_div < 0 || duty_percent < 0 || duty_percent > 100)
+   return -EINVAL;
+
+   err = pwm->chip->ops->config_alternate(pwm->chip, pwm, clk_div, 
duty_percent);
+   if (err)
+   return err;
+
+   pwm->clk_div = clk_div;
+   pwm->duty_percent = duty_percent;
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(pwm_config_alternate);
+
+/**
  * pwm_set_polarity() - configure the polarity of a PWM signal
  * @pwm: PWM device
  * @polarity: new polarity of the PWM signal
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index e90628c..739cb2b 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -24,6 +24,12 @@ void pwm_free(struct pwm_device *pwm);
 int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns);
 
 /*
+ * pwm_config_alternate - change a PWM device configuration
+ *   based on clk_dic and duty_percent
+ */
+int pwm_config_alternate(struct pwm_device *pwm, int clk_div, int 
duty_percent);
+
+/*
  * pwm_enable - start a PWM output toggling
  */
 int pwm_enable(struct pwm_device *pwm);
@@ -89,6 +95,8 @@ struct pwm_device {
 
unsigned intperiod; /* in nanoseconds */
unsigned intduty_cycle; /* in nanoseconds */
+   unsigned intclk_div;
+   unsigned intduty_percent;
enum pwm_polarity   polarity;
 };
 
@@ -114,6 +122,28 @@ static inline unsigned int pwm_get_duty_cycle(struct 
pwm_device *pwm)
return pwm ? pwm->duty_cycle : 0;
 }
 
+static inline void pwm_set_clk_div(struct pwm_device *pwm, unsigned int 
clk_div)
+{
+   if (pwm)
+   pwm->clk_div = clk_div;
+}
+
+static inline unsigned int pwm_get_clk_div(struct pwm_device *pwm)
+{
+   return pwm ? pwm->clk_div : 0;
+}
+
+static inline void pwm_set_duty_percent(struct pwm_device *pwm, unsigned int 
duty_percent)
+{
+   if (pwm)
+   pwm->duty_percent = duty_percent;
+}
+
+static inline unsigned int pwm_get_duty_percent(struct pwm_device *pwm)
+{
+   return pwm ? pwm->duty_percent : 0;
+}
+
 /*
  * pwm_set_polarity - configure the polarity of a PWM signal
  */
@@ -138,6 +168,9 @@ struct pwm_ops {
int (*config)(struct pwm_chip *chip,
  struct pwm_device *pwm,
  int duty_ns, int period_ns);
+   int (*config_alternate)(struct pwm_chip *chip,
+ struct pwm_device *pwm,
+ int clk_div, int duty_percent);
int (*set_polarity)(struct pwm_chip *chip,
  struct pwm_device *pwm,
  enum pwm_polarity polarity);
-- 
2.1.0

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

[Intel-gfx] [RFC v5 2/9] gpio/crystalcove: Add additional GPIO for Panel control

[Shobhit Kumar]

Export PANEL_EN/DISABLE (offset 0x52) as additional GPIO. Needed
by display driver to enable the DSI panel on BYT platform where
the Panel EN/Disable control is routed thorugh CRC PMIC

CC: Samuel Ortiz 
Cc: Linus Walleij 
Cc: Alexandre Courbot 
Cc: Thierry Reding 
Signed-off-by: Shobhit Kumar 
---
 drivers/gpio/gpio-crystalcove.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-crystalcove.c b/drivers/gpio/gpio-crystalcove.c
index 3d9e08f..91a7ffe 100644
--- a/drivers/gpio/gpio-crystalcove.c
+++ b/drivers/gpio/gpio-crystalcove.c
@@ -24,7 +24,7 @@
 #include 
 
 #define CRYSTALCOVE_GPIO_NUM   16
-#define CRYSTALCOVE_VGPIO_NUM  94
+#define CRYSTALCOVE_VGPIO_NUM  95
 
 #define UPDATE_IRQ_TYPEBIT(0)
 #define UPDATE_IRQ_MASKBIT(1)
@@ -39,6 +39,7 @@
 #define GPIO0P0CTLI0x33
 #define GPIO1P0CTLO0x3b
 #define GPIO1P0CTLI0x43
+#define GPIOPANELCTL   0x52
 
 #define CTLI_INTCNT_DIS(0)
 #define CTLI_INTCNT_NE (1 << 1)
@@ -93,6 +94,10 @@ static inline int to_reg(int gpio, enum ctrl_register 
reg_type)
 {
int reg;
 
+   if (gpio == 94) {
+   return GPIOPANELCTL;
+   }
+
if (reg_type == CTRL_IN) {
if (gpio < 8)
reg = GPIO0P0CTLI;
-- 
2.1.0

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

[Intel-gfx] [RFC v5 0/9] Crystalcove (CRC) PMIC based panel and pwm control

[Shobhit Kumar]

Hi All,
These patches try to enable panel control as a new GPIO via lookup table
during pmic core driver loading. Code is modifed to use generic gpiod_*
routines which will take care of ownership issues. 

Also a new PWM cell device is added to the PMIC MFD. New PWM driver pwm-crc
is used to do pwm control. Since the pwm is to be configured using clock
divisor and duty percent instead of period_ns and duty_ns new helpers have been
attempted in pwm core framework. Again pwm is added to a lookup table during
PMIC core driver load time.

Finally all is tied in the display driver enabling DSI panel. As of now the pwm
use is not hooked in to the bigger backlight class driver based infra in i915
but will be done as next step.

Sending all patches together to give a complete view of changes and how they gel
together. Please provide your comments.

Regards
Shobhit

Shobhit Kumar (9):
  drivers/mfd: Add lookup table for Panel Control as GPIO signal
  gpio/crystalcove: Add additional GPIO for Panel control
  drm/i915: Use the CRC gpio for panel enable/disable
  drivers/pwm: Add helper to configure pwm using clock divisor and duty
percent
  drivers/mfd: Add PWM cell device for Crystalcove PMIC
  drivers/pwm: Add Crystalcove (CRC) PWM driver
  drivers/pwm: Remove __init initializer for pwm_add_table
  drivers/mfd: ADD PWM lookup table for CRC PMIC based PWM
  drm/i915: Backlight control using CRC PMIC based PWM driver

 drivers/gpio/gpio-crystalcove.c   |   7 +-
 drivers/gpu/drm/i915/intel_dsi.c  |  59 +-
 drivers/gpu/drm/i915/intel_dsi.h  |  13 +++
 drivers/mfd/intel_soc_pmic_core.c |  23 ++
 drivers/mfd/intel_soc_pmic_crc.c  |   3 +
 drivers/pwm/Kconfig   |   7 ++
 drivers/pwm/Makefile  |   1 +
 drivers/pwm/core.c|  26 +-
 drivers/pwm/pwm-crc.c | 161 ++
 include/linux/pwm.h   |  33 
 10 files changed, 329 insertions(+), 4 deletions(-)
 create mode 100644 drivers/pwm/pwm-crc.c

-- 
2.1.0

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

[Intel-gfx] [RFC v5 9/9] drm/i915: Backlight control using CRC PMIC based PWM driver

[Shobhit Kumar]

CC: Samuel Ortiz 
Cc: Linus Walleij 
Cc: Alexandre Courbot 
Cc: Thierry Reding 
Signed-off-by: Shobhit Kumar 
---
 drivers/gpu/drm/i915/intel_dsi.c | 25 +
 drivers/gpu/drm/i915/intel_dsi.h |  3 +++
 2 files changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 219421c..511446f 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "i915_drv.h"
 #include "intel_drv.h"
 #include "intel_dsi.h"
@@ -402,6 +403,10 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
 
intel_dsi_port_enable(encoder);
}
+
+   /* Enable the backlight with default PWM as programmed by BIOS */
+   pwm_enable(intel_dsi->pwm);
+   pwm_config_alternate(intel_dsi->pwm, 0x7F, 100);
 }
 
 static void intel_dsi_pre_enable(struct intel_encoder *encoder)
@@ -467,6 +472,11 @@ static void intel_dsi_pre_disable(struct intel_encoder 
*encoder)
 
DRM_DEBUG_KMS("\n");
 
+   /* Disable the backlight */
+   pwm_config_alternate(intel_dsi->pwm, 0x7F, 0);
+   usleep_range(2000, 3000);
+   pwm_disable(intel_dsi->pwm);
+
if (is_vid_mode(intel_dsi)) {
/* Send Shutdown command to the panel in LP mode */
for_each_dsi_port(port, intel_dsi->ports)
@@ -971,6 +981,10 @@ static void intel_dsi_encoder_destroy(struct drm_encoder 
*encoder)
if (intel_dsi->gpio_panel)
gpiod_put(intel_dsi->gpio_panel);
 
+   /* dispose of the pwm */
+   if (intel_dsi->pwm)
+   pwm_put(intel_dsi->pwm);
+
intel_encoder_destroy(encoder);
 }
 
@@ -1098,6 +1112,17 @@ void intel_dsi_init(struct drm_device *dev)
DRM_ERROR("Failed to own gpio for panel control\n");
intel_dsi->gpio_panel = NULL;
}
+
+   /* Get the PWM chip for backlight control */
+   intel_dsi->pwm = pwm_get(dev->dev, "pwm_backlight");
+   if (IS_ERR(intel_dsi->pwm)) {
+   DRM_ERROR("Faild to own the pwm chip\n");
+   intel_dsi->pwm = NULL;
+   } else if (pwm_config_alternate(intel_dsi->pwm, 0x7F, 100) < 0) 
{
+   DRM_ERROR("Failed to configure the pwm chip\n");
+   pwm_put(intel_dsi->pwm);
+   intel_dsi->pwm = NULL;
+   }
}
 
intel_encoder->type = INTEL_OUTPUT_DSI;
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 8be75a7..d8ec9c1 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -52,6 +52,9 @@ struct intel_dsi {
/* GPIO Desc for CRC based Panel control */
struct gpio_desc *gpio_panel;
 
+   /* PWM chip */
+   struct pwm_device *pwm;
+
struct intel_connector *attached_connector;
 
/* bit mask of ports being driven */
-- 
2.1.0

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

Re: [Intel-gfx] [PATCH v3] drm/i915: Optimistically spin for the request completion

[Tvrtko Ursulin]

On 03/12/2015 04:28 PM, Chris Wilson wrote:

On Thu, Mar 12, 2015 at 03:18:01PM +, Tvrtko Ursulin wrote:

On 03/12/2015 01:18 PM, Chris Wilson wrote:

1ms. I was just thinking of doing USECS_PER_SEC / HZ, then realised that
was a jiffie, hence the confusion. At any rate, it is still the minimum
we can trivially wait for (without an expensive hrtimer).


Unless I lost track with the times, that's CONFIG_HZ right?

I don't know what server distributions do, but this Ubuntu LTS I am
running has HZ=250 which means 4ms.

That would mean on a system where throughput is more important than
latency, you lose most throughput by spinning the longest. In theory
at least, no?


Only in theory, and only if you mean throughput of non-i915 workloads
with preemption disabled.  Spinning here improves both latency and
throughput for gfx clients. Using up the timeslice for the client may
have secondary effects though - otherwise they would get iowait credits.


Yes, I meant CPU workloads. And low HZ and no preemption usually go 
together.



So perhaps which should be a tunable? Optionally auto-select the
initial state based on HZ.


Spinning less than a jiffie requires hrtimers at which point you may as
well just use the i915 interrupt (rather than setup a timer interrupt).


Yes I didn't mean that - but to have a boolean spinning-wait=on/off. 
Maybe default to "on" on HZ=1000 with preemption, or the opposite, 
something like that.


Regards,

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

Re: [Intel-gfx] [PATCH v3] drm/i915: Optimistically spin for the request completion

[Chris Wilson]

On Thu, Mar 12, 2015 at 04:41:10PM +, Tvrtko Ursulin wrote:
> Yes I didn't mean that - but to have a boolean spinning-wait=on/off.
> Maybe default to "on" on HZ=1000 with preemption, or the opposite,
> something like that.

I don't see the point in having the complication, until someone
complains. In my defense, I will point to the optimistic mutex spinning
equally having no configurable option. And since the idea is that you
only hit this if you are abusing i915 (e.g. benchmarking, or you have a
readback on the critical patch, or if we haven't implemented
semaphores), I would rather fix those scenarios as they arrive rather
than giving the user an option to break.
-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: redefine WARN_ON_ONCE to include the condition

[shuang . he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: 
shuang...@intel.com)
Task id: 5940
-Summary-
Platform  Delta  drm-intel-nightly  Series Applied
PNV -6  274/274  268/274
ILK  301/301  301/301
SNB -1  277/277  276/277
IVB  341/341  341/341
BYT  287/287  287/287
HSW  360/360  360/360
BDW  308/308  308/308
-Detailed-
Platform  Testdrm-intel-nightly  Series 
Applied
*PNV  igt_gem_userptr_blits_minor-normal-sync  PASS(4)  
DMESG_WARN(1)PASS(1)
 PNV  igt_gen3_render_linear_blits  FAIL(3)PASS(3)  FAIL(2)
 PNV  igt_gen3_render_mixed_blits  FAIL(3)PASS(3)  FAIL(2)
 PNV  igt_gen3_render_tiledx_blits  FAIL(5)PASS(2)  FAIL(2)
 PNV  igt_gen3_render_tiledy_blits  FAIL(5)PASS(1)  FAIL(2)
 PNV  igt_gem_tiled_pread_pwrite  FAIL(2)PASS(2)  FAIL(2)
*SNB  igt_gem_mmap_short-mmap  PASS(2)  DMESG_WARN(1)PASS(1)
Note: You need to pay more attention to line start with '*'
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [ANNOUNCE] intel-gpu-tools 1.10

[Thomas Wood]

A new intel-gpu-tools quarterly release is available with the following changes:

- New frequency manipulation tool (intel_gpu_frequency)

- Adjustments for the Solaris port (Alan Coopersmith).

- Remove tests/NAMING-CONVENTION since it's all in the docbook now, to avoid
  divergent conventions.

- New CRITICAL log level for really serious stuff (Thomas Wood).

- Interactive test mode can now be enabled by the shared cmdline option
  --interactive-debug=$var (Rodrigo Vivi).

- Improved logging to kmsg to better line up test runs with kernel messages
  (Chris Wilson).

- Record all log levels (including disabled levels) in a ringbuffer and dump
  that on test failures for quicker diagnostics of automated test run results
  (Thomas Wood).

- A lot of small polish all over the test library.

- Piles of new testcases and improvements to existing ones as usual.


Complete list of changes since 1.9:

Akash Goel (1):
  igt/gem_mmap_wc: Add the invalid flags subtest

Alan Coopersmith (7):
  Fix #ifdef check for _SC_AVPHYS_PAGES in intel_get_avail_ram_mb()
  Solaris needs to #include  instead of 
  Use pthread calls instead of raw syscalls on non-Linux systems
  Need to #include  for basename() on Solaris
  Provide sighandler_t fallback for non-GNU-libc platforms
  Don't try to use CLOCK_MONOTONIC_COARSE on OS'es that don't support it
  Skip MADV_DOFORK & MADV_DONTFORK calls on OS'es that don't support them

Ander Conselvan de Oliveira (1):
  kms_plane: Add test that suspends/resumes before getting crc

Ben Widawsky (3):
  tools/Makefile: Alphabetize the list
  intel_gpu_frequency: A tool to manipulate Intel GPU frequency
  gem_render_copy: Provide an all pixels check

Chris Wilson (38):
  igt: Add gem_ctx_thrash to fill the GGTT with contexts
  lib: random() is too slow
  lib/gen8: Make rendercopy threadsafe
  igt/gem_ctx_thrash: Boost workloads
  igt/gem_ctx_thread/processes: Serialise after forking children
  igt/gem_ctx_thrash/threads: Allow bo resuse
  overlay: Negative modulus
  overlay: A couple of valgrind pleasers
  overlay: Hide kworker threads in overview
  lib/core: Show the exitcode in kmsg as well
  lib/core: Fix compile error from rebasing
  igt/gem_mmap_wc: Exercise mmap(wc) interface
  igt/gem_tiled_wc: Exercise wc mmaps with swizzling
  igt/gem_gtt_speed: compare against WC mmaps
  igt/gem_fence_upload: Add comparison against wc mmaps
  igt/gem_concurrent_blit: Exercise wc mappings
  ioct_wrappers: Add some mmap(wc) blurb dropped between authors
  igt/gem_ctx_thrash: Tweak resource limits
  igt/gem_concurrent_blit: Inject hangs before verifying contents
  igt/gem_pread_after_blit: Inject hangs!
  igt/gem_reloc_vs_hang: Inject hangs!
  igt/gem_evict_(alignment|everything): contend with GPU hangs
  igt/gem_exec_big: Increase stress
  igt/gem_exec_big: Also test a large batch with a large number of relocs
  igt/gem_exec_big: Use mmap(wc) to speed up verification
  igt/gem_mmap_wc: Reorder gem_close()
  igt/gem_exec_big: Don't try to repeatedly munmap(NULL)
  igt/gem_mmap_wc/set-cache-level: Exercise set-cache-level WARNing
  igt/gem_tiled_swapping: Cycle through the bo a couple of times
  igt/drv_module_reload: Check more carefully for a live driver
  igt/gem_tiled_wc: Fix! Finish!
  igt/gem_tiled_wc: Use correct offsets
  lib: Cache DRM device id to reduce number of ioctls
  lib: Use strtol not strtod for overiding the PCI ID
  igt/gem_wait: Timeout parameter to the WAIT ioctl is signed
  igt/gem_wait: Test negative timeouts
  lib/core: Make the start of the debug output more clear
  igt/kms_psr_sink_crc: Prettify i915_edp_psr_status failures

Damien Lespiau (8):
  quick_dump: Add interrupt and PPAT registers to the SKL dump
  lib/skl: Add gen9 specific igt_blitter_fast_copy()
  lib: Don't give a struct igt_buf * to fast_copy_pitch()
  lib: Split two helpers to build fast copy's dword0 and dword1
  lib: Provide a raw version of the gen9 fast copy blits
  lib: Allow the creation of Ys/Yf tiled FBs
  testdisplay/skl: Add command line options for Yb/Yf tiled fbs
  lib/fb: Use PRIx64 for uint64_t in format string

Daniel Vetter (27):
  tests/gem_exec_blt: Add subtest that uses dumb buffers
  tests/gem_concurrent_blt: Adjust subtest naming
  tests/gem_concurrent_blit: Fix indent
  lib/gt: api polish for igt_can_hang_ring
  lib/ioctl: api polish for gem_context_has_param
  lib/ioctl: gem_ prefix for igt_require_mmap_wc
  igt/ioctls: doc for gem_mmap
  lib/ioctls: make gem_context_set/get_param infallible
  lib/ioctl: Add gem_context_destroy helpers
  tests/gem_ctx_*: Use helpers
  tests/gem_reset_stat: Use new ctx helpers
  lib/ioctl: Document ctx param functions
  tests: Add gem_ctx_param_basic
  tests: Add invalid pad te

Re: [Intel-gfx] [PATCH v3] drm/i915: Optimistically spin for the request completion

[Tvrtko Ursulin]

On 03/12/2015 04:50 PM, Chris Wilson wrote:

On Thu, Mar 12, 2015 at 04:41:10PM +, Tvrtko Ursulin wrote:

Yes I didn't mean that - but to have a boolean spinning-wait=on/off.
Maybe default to "on" on HZ=1000 with preemption, or the opposite,
something like that.


I don't see the point in having the complication, until someone
complains. In my defense, I will point to the optimistic mutex spinning
equally having no configurable option. And since the idea is that you
only hit this if you are abusing i915 (e.g. benchmarking, or you have a
readback on the critical patch, or if we haven't implemented
semaphores), I would rather fix those scenarios as they arrive rather
than giving the user an option to break.


I simply pointed out a theoretical potential to burn more CPU on 
servers. If you know that is unlikely or only theoretical that's fine by me.


But I'll say I was more convinced before you mentioned "until someone 
complains" and "option to break". :)


Regards,

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

[Intel-gfx] [PATCH i-g-t 1/2 v2] lib: Add media spin

[jeff . mcgee]

From: Jeff McGee 

The media spin utility is derived from media fill. The purpose
is to create a simple means to keep the render engine (media
pipeline) busy for a controlled amount of time. It does so by
emitting a batch with a single execution thread that spins in
a tight loop the requested number of times. Each spin increments
a counter whose final 32-bit value is written to the destination
buffer on completion for checking. The implementation supports
Gen8, Gen8lp, and Gen9.

v2: Apply the recommendations of igt.cocci.

Signed-off-by: Jeff McGee 
---
 lib/Makefile.sources|   2 +
 lib/intel_batchbuffer.c |  24 +++
 lib/intel_batchbuffer.h |  22 ++
 lib/media_spin.c| 540 
 lib/media_spin.h|  39 
 5 files changed, 627 insertions(+)
 create mode 100644 lib/media_spin.c
 create mode 100644 lib/media_spin.h

diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 76f353a..3d93629 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -29,6 +29,8 @@ libintel_tools_la_SOURCES =   \
media_fill_gen8.c   \
media_fill_gen8lp.c \
media_fill_gen9.c   \
+   media_spin.h\
+   media_spin.c\
gen7_media.h\
gen8_media.h\
rendercopy_i915.c   \
diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
index 666c323..195ccc4 100644
--- a/lib/intel_batchbuffer.c
+++ b/lib/intel_batchbuffer.c
@@ -40,6 +40,7 @@
 #include "rendercopy.h"
 #include "media_fill.h"
 #include "ioctl_wrappers.h"
+#include "media_spin.h"
 
 #include 
 
@@ -785,3 +786,26 @@ igt_fillfunc_t igt_get_gpgpu_fillfunc(int devid)
 
return fill;
 }
+
+/**
+ * igt_get_media_spinfunc:
+ * @devid: pci device id
+ *
+ * Returns:
+ *
+ * The platform-specific media spin function pointer for the device specified
+ * with @devid. Will return NULL when no media spin function is implemented.
+ */
+igt_media_spinfunc_t igt_get_media_spinfunc(int devid)
+{
+   igt_media_spinfunc_t spin = NULL;
+
+   if (IS_GEN9(devid))
+   spin = gen9_media_spinfunc;
+   else if (IS_BROADWELL(devid))
+   spin = gen8_media_spinfunc;
+   else if (IS_CHERRYVIEW(devid))
+   spin = gen8lp_media_spinfunc;
+
+   return spin;
+}
diff --git a/lib/intel_batchbuffer.h b/lib/intel_batchbuffer.h
index fa8875b..62c8396 100644
--- a/lib/intel_batchbuffer.h
+++ b/lib/intel_batchbuffer.h
@@ -300,4 +300,26 @@ typedef void (*igt_fillfunc_t)(struct intel_batchbuffer 
*batch,
 igt_fillfunc_t igt_get_media_fillfunc(int devid);
 igt_fillfunc_t igt_get_gpgpu_fillfunc(int devid);
 
+/**
+ * igt_media_spinfunc_t:
+ * @batch: batchbuffer object
+ * @dst: destination i-g-t buffer object
+ * @spins: number of loops to execute
+ *
+ * This is the type of the per-platform media spin functions. The
+ * platform-specific implementation can be obtained by calling
+ * igt_get_media_spinfunc().
+ *
+ * The media spin function emits a batchbuffer for the render engine with
+ * the media pipeline selected. The workload consists of a single thread
+ * which spins in a tight loop the requested number of times. Each spin
+ * increments a counter whose final 32-bit value is written to the
+ * destination buffer on completion. This utility provides a simple way
+ * to keep the render engine busy for a set time for various tests.
+ */
+typedef void (*igt_media_spinfunc_t)(struct intel_batchbuffer *batch,
+struct igt_buf *dst, uint32_t spins);
+
+igt_media_spinfunc_t igt_get_media_spinfunc(int devid);
+
 #endif
diff --git a/lib/media_spin.c b/lib/media_spin.c
new file mode 100644
index 000..580c109
--- /dev/null
+++ b/lib/media_spin.c
@@ -0,0 +1,540 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ * Jeff

[Intel-gfx] [PATCH i-g-t 2/2 v2] tests/pm_sseu: Create new test pm_sseu

[jeff . mcgee]

From: Jeff McGee 

New test pm_sseu is intended for any subtest related to the
slice/subslice/EU power gating feature. The sole initial subtest,
'full-enable', confirms that the slice/subslice/EU state is at
full enablement when the render engine is active. Starting with
Gen9 SKL, the render power gating feature can leave SSEU in a
partially enabled state upon resumption of render work unless
explicit action is taken.

v2: Add test description and apply recommendations of igt.cocci
(Thomas Wood).

Signed-off-by: Jeff McGee 
---
 tests/.gitignore   |   1 +
 tests/Makefile.sources |   1 +
 tests/pm_sseu.c| 375 +
 3 files changed, 377 insertions(+)
 create mode 100644 tests/pm_sseu.c

diff --git a/tests/.gitignore b/tests/.gitignore
index 426cc67..a1ec1b5 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -143,6 +143,7 @@ pm_lpsp
 pm_rc6_residency
 pm_rpm
 pm_rps
+pm_sseu
 prime_nv_api
 prime_nv_pcopy
 prime_nv_test
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 51e8376..74106c0 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -82,6 +82,7 @@ TESTS_progs_M = \
pm_rpm \
pm_rps \
pm_rc6_residency \
+   pm_sseu \
prime_self_import \
template \
$(NULL)
diff --git a/tests/pm_sseu.c b/tests/pm_sseu.c
new file mode 100644
index 000..7196dcb
--- /dev/null
+++ b/tests/pm_sseu.c
@@ -0,0 +1,375 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Jeff McGee 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "drmtest.h"
+#include "i915_drm.h"
+#include "intel_io.h"
+#include "intel_bufmgr.h"
+#include "intel_batchbuffer.h"
+#include "intel_chipset.h"
+#include "ioctl_wrappers.h"
+#include "igt_debugfs.h"
+#include "media_spin.h"
+
+IGT_TEST_DESCRIPTION("Tests slice/subslice/EU power gating functionality.\n");
+
+static double
+to_dt(const struct timespec *start, const struct timespec *end)
+{
+   double dt;
+
+   dt = (end->tv_sec - start->tv_sec) * 1e3;
+   dt += (end->tv_nsec - start->tv_nsec) * 1e-6;
+
+   return dt;
+}
+
+struct status {
+   struct {
+   int slice_total;
+   int subslice_total;
+   int subslice_per;
+   int eu_total;
+   int eu_per;
+   bool has_slice_pg;
+   bool has_subslice_pg;
+   bool has_eu_pg;
+   } info;
+   struct {
+   int slice_total;
+   int subslice_total;
+   int subslice_per;
+   int eu_total;
+   int eu_per;
+   } hw;
+};
+
+#define DBG_STATUS_BUF_SIZE 4096
+
+struct {
+   int init;
+   int status_fd;
+   char status_buf[DBG_STATUS_BUF_SIZE];
+} dbg;
+
+static void
+dbg_get_status_section(const char *title, char **first, char **last)
+{
+   char *pos;
+
+   *first = strstr(dbg.status_buf, title);
+   igt_assert(*first != NULL);
+
+   pos = *first;
+   do {
+   pos = strchr(pos, '\n');
+   igt_assert(pos != NULL);
+   pos++;
+   } while (*pos == ' '); /* lines in the section begin with a space */
+   *last = pos - 1;
+}
+
+static int
+dbg_get_int(const char *first, const char *last, const char *name)
+{
+   char *pos;
+
+   pos = strstr(first, name);
+   igt_assert(pos != NULL);
+   pos = strstr(pos, ":");
+   igt_assert(pos != NULL);
+   pos += 2;
+   igt_assert(pos != last);
+
+   return strtol(pos, &pos, 10);
+}
+
+static bool
+dbg_get_bool(const char *first, const char *last, const char *name)
+{
+   char *pos;
+
+   pos = strstr(first, name);
+   igt_assert(pos != NULL);
+   pos = strstr(pos, ":");
+   igt_assert(pos != NULL);
+   pos += 2;
+   igt_assert(po

Re: [Intel-gfx] [PATCH v2] drm/i915/skl: Implement WaDisableHBR2

[Jesse Barnes]

On 02/11/2015 09:43 AM, Damien Lespiau wrote:
> v2: Use the recently introduced INTEL_REVID() and SKL_REVID defines
> (Nick Hoath)
> 
> Signed-off-by: Damien Lespiau 
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d4c82d7..a7bc3e8 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -129,7 +129,10 @@ intel_dp_max_link_bw(struct intel_dp *intel_dp)
>   case DP_LINK_BW_2_7:
>   break;
>   case DP_LINK_BW_5_4: /* 1.2 capable displays may advertise higher bw */
> - if (((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) ||
> + if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
> + /* WaDisableHBR2:skl */
> + max_link_bw = DP_LINK_BW_2_7;
> + else if (((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) ||
>INTEL_INFO(dev)->gen >= 8) &&
>   intel_dp->dpcd[DP_DPCD_REV] >= 0x12)
>   max_link_bw = DP_LINK_BW_5_4;
> 

Reviewed-by: Jesse Barnes 
Tested-by: Jesse Barnes 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915/skl: Implement WaDisableHBR2

[Jani Nikula]

On Thu, 12 Mar 2015, Jesse Barnes  wrote:
> On 02/11/2015 09:43 AM, Damien Lespiau wrote:
>> v2: Use the recently introduced INTEL_REVID() and SKL_REVID defines
>> (Nick Hoath)
>> 
>> Signed-off-by: Damien Lespiau 
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index d4c82d7..a7bc3e8 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -129,7 +129,10 @@ intel_dp_max_link_bw(struct intel_dp *intel_dp)
>>  case DP_LINK_BW_2_7:
>>  break;
>>  case DP_LINK_BW_5_4: /* 1.2 capable displays may advertise higher bw */
>> -if (((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) ||
>> +if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
>> +/* WaDisableHBR2:skl */
>> +max_link_bw = DP_LINK_BW_2_7;
>> +else if (((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) ||
>>   INTEL_INFO(dev)->gen >= 8) &&
>>  intel_dp->dpcd[DP_DPCD_REV] >= 0x12)
>>  max_link_bw = DP_LINK_BW_5_4;
>> 
>
> Reviewed-by: Jesse Barnes 
> Tested-by: Jesse Barnes 

Potentially

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


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

-- 
Jani Nikula, 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/skl: Implement WaDisableHBR2

[Jesse Barnes]

On 03/12/2015 11:36 AM, Jani Nikula wrote:
> On Thu, 12 Mar 2015, Jesse Barnes  wrote:
>> On 02/11/2015 09:43 AM, Damien Lespiau wrote:
>>> v2: Use the recently introduced INTEL_REVID() and SKL_REVID defines
>>> (Nick Hoath)
>>>
>>> Signed-off-by: Damien Lespiau 
>>> ---
>>>  drivers/gpu/drm/i915/intel_dp.c | 5 -
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>>> b/drivers/gpu/drm/i915/intel_dp.c
>>> index d4c82d7..a7bc3e8 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -129,7 +129,10 @@ intel_dp_max_link_bw(struct intel_dp *intel_dp)
>>> case DP_LINK_BW_2_7:
>>> break;
>>> case DP_LINK_BW_5_4: /* 1.2 capable displays may advertise higher bw */
>>> -   if (((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) ||
>>> +   if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
>>> +   /* WaDisableHBR2:skl */
>>> +   max_link_bw = DP_LINK_BW_2_7;
>>> +   else if (((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) ||
>>>  INTEL_INFO(dev)->gen >= 8) &&
>>> intel_dp->dpcd[DP_DPCD_REV] >= 0x12)
>>> max_link_bw = DP_LINK_BW_5_4;
>>>
>>
>> Reviewed-by: Jesse Barnes 
>> Tested-by: Jesse Barnes 
> 
> Potentially
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89554

Yeah hopefully.  I definitely need it here for eDP to come up reliably.

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

Re: [Intel-gfx] [PATCH v3] drm/i915: Optimistically spin for the request completion

[shuang . he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: 
shuang...@intel.com)
Task id: 5941
-Summary-
Platform  Delta  drm-intel-nightly  Series Applied
PNV -7  274/274  267/274
ILK  301/301  301/301
SNB -1  277/277  276/277
IVB  341/341  341/341
BYT  287/287  287/287
HSW  360/360  360/360
BDW  308/308  308/308
-Detailed-
Platform  Testdrm-intel-nightly  Series 
Applied
*PNV  igt_gem_fence_thrash_bo-write-verify-none  PASS(4)  FAIL(1)PASS(1)
*PNV  igt_gem_fence_thrash_bo-write-verify-x  PASS(4)  FAIL(1)PASS(1)
*PNV  igt_gem_fence_thrash_bo-write-verify-y  PASS(4)  FAIL(1)PASS(1)
 PNV  igt_gem_userptr_blits_coherency-sync  CRASH(5)PASS(4)  
CRASH(1)PASS(1)
 PNV  igt_gem_userptr_blits_coherency-unsync  CRASH(1)PASS(3)  
CRASH(1)PASS(1)
 PNV  igt_gen3_render_linear_blits  FAIL(3)PASS(4)  FAIL(1)PASS(1)
 PNV  igt_gen3_render_mixed_blits  FAIL(3)PASS(4)  FAIL(2)
*SNB  igt_gem_mmap_gtt_write-read  PASS(2)  DMESG_WARN(1)PASS(1)
Note: You need to pay more attention to line start with '*'
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 13/13] drm/i915: Include the sink/source/supported rates in debug output

[shuang . he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: 
shuang...@intel.com)
Task id: 5942
-Summary-
Platform  Delta  drm-intel-nightly  Series Applied
PNV -2  275/275  273/275
ILK  302/302  302/302
SNB  281/281  281/281
IVB  341/341  341/341
BYT  287/287  287/287
HSW -1  361/361  360/361
BDW  308/308  308/308
-Detailed-
Platform  Testdrm-intel-nightly  Series 
Applied
*PNV  igt_gem_userptr_blits_minor-normal-sync  PASS(2)  DMESG_WARN(2)
*PNV  igt_gem_userptr_blits_minor-unsync-normal  PASS(4)  DMESG_WARN(2)
*HSW  igt_gem_storedw_loop_blt  PASS(2)  DMESG_WARN(1)PASS(1)
Note: You need to pay more attention to line start with '*'
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t v2] tests/core_getparams: Create new test core_getparams

[jeff . mcgee]

From: Jeff McGee 

New test core_getparams consists of 2 subtests, each one testing
the ability of userspace to query the correct value of a GT config
attribute: subslice total or EU total. drm/i915 implementation of
these queries is required for Cherryview and Gen9+ devices (non-
simulated).

v2: Duplicate small amount of new libdrm functionality to avoid
bumping libdrm version requirement (Daniel). Convert some
igt_asserts to the appropriate comparison variants. Add a
test description.

For: VIZ-4636
Signed-off-by: Jeff McGee 
---
 tests/.gitignore   |   1 +
 tests/Makefile.sources |   1 +
 tests/core_getparams.c | 167 +
 3 files changed, 169 insertions(+)
 create mode 100644 tests/core_getparams.c

diff --git a/tests/.gitignore b/tests/.gitignore
index 426cc67..c742308 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -1,6 +1,7 @@
 # Please keep sorted alphabetically
 core_get_client_auth
 core_getclient
+core_getparams
 core_getstats
 core_getversion
 drm_import_export
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 51e8376..999c8f8 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -15,6 +15,7 @@ NOUVEAU_TESTS_M = \
 
 TESTS_progs_M = \
core_get_client_auth \
+   core_getparams \
drv_suspend \
drv_hangman \
gem_bad_reloc \
diff --git a/tests/core_getparams.c b/tests/core_getparams.c
new file mode 100644
index 000..2855d06
--- /dev/null
+++ b/tests/core_getparams.c
@@ -0,0 +1,167 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Jeff McGee 
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include "drmtest.h"
+#include "intel_chipset.h"
+#include "intel_bufmgr.h"
+
+IGT_TEST_DESCRIPTION("Tests the export of parameters via 
DRM_IOCTL_I915_GETPARAM\n");
+
+int drm_fd;
+int devid;
+
+static void
+init(void)
+{
+   drm_fd = drm_open_any();
+   devid = intel_get_drm_devid(drm_fd);
+}
+
+static void
+deinit(void)
+{
+   close(drm_fd);
+}
+
+#define LOCAL_I915_PARAM_SUBSLICE_TOTAL33
+#define LOCAL_I915_PARAM_EU_TOTAL  34
+
+static int
+getparam(int param, int *value)
+{
+   drm_i915_getparam_t gp;
+   int ret;
+
+   memset(&gp, 0, sizeof(gp));
+   gp.value = value;
+   gp.param = param;
+   ret = drmIoctl(drm_fd, DRM_IOCTL_I915_GETPARAM, &gp);
+   if (ret)
+   return -errno;
+
+   return 0;
+}
+
+static void
+subslice_total(void)
+{
+   unsigned int subslice_total = 0;
+   int ret;
+
+   ret = getparam(I915_PARAM_SUBSLICE_TOTAL, (int*)&subslice_total);
+
+   if (ret) {
+   /*
+* These devices are not required to implement the
+* interface. If they do not, -ENODEV must be returned.
+   */
+   if ((intel_gen(devid) < 8) ||
+   IS_BROADWELL(devid) ||
+   igt_run_in_simulation()) {
+   igt_assert_eq(ret, -ENODEV);
+   igt_info("subslice total: unknown\n");
+   /*
+* All other devices must implement the interface, so
+* fail them if we are here.
+   */
+   } else {
+   igt_assert_neq(ret, EINVAL); /* request not recognized? 
*/
+   igt_assert_neq(ret, ENODEV); /* device not supported? */
+   igt_assert_eq(ret, 0); /* other error? */
+   }
+   } else {
+   /*
+* On success, just make sure the returned count value is
+* non-zero. The validity of the count value for the given
+* device is not checked.
+   */
+   igt_assert_neq(subslice_total, 0);
+   igt_info("subslice total: %u\n", su

Re: [Intel-gfx] [Beignet] [PATCH i-g-t 2/2] configure: Bump required libdrm version to 2.4.60

[Jeff McGee]

On Wed, Mar 11, 2015 at 08:21:36AM +0100, Daniel Vetter wrote:
> On Tue, Mar 10, 2015 at 01:06:44PM -0700, Jeff McGee wrote:
> > On Tue, Mar 10, 2015 at 07:47:03PM +0100, Daniel Vetter wrote:
> > > On Tue, Mar 10, 2015 at 01:58:52PM -0400, Rob Clark wrote:
> > > > On Tue, Mar 10, 2015 at 12:59 PM, Jeff McGee  
> > > > wrote:
> > > > > On Tue, Mar 10, 2015 at 08:37:30AM +0100, Daniel Vetter wrote:
> > > > >> On Mon, Mar 09, 2015 at 04:41:02PM -0700, jeff.mc...@intel.com wrote:
> > > > >> > From: Jeff McGee 
> > > > >> >
> > > > >> > tests/core_getparams needs the new libdrm interfaces for
> > > > >> > querying subslice and EU counts.
> > > > >> >
> > > > >> > For: VIZ-4636
> > > > >> > Signed-off-by: Jeff McGee 
> > > > >> > ---
> > > > >> >  configure.ac | 2 +-
> > > > >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >> >
> > > > >> > diff --git a/configure.ac b/configure.ac
> > > > >> > index 16d6a2e..88a1c3d 100644
> > > > >> > --- a/configure.ac
> > > > >> > +++ b/configure.ac
> > > > >> > @@ -82,7 +82,7 @@ if test "x$GCC" = "xyes"; then
> > > > >> >  fi
> > > > >> >  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
> > > > >> >
> > > > >> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.52 libdrm])
> > > > >> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.60 libdrm])
> > > > >>
> > > > >> Please don't and instead copypaste the new structs/defines with a 
> > > > >> local_
> > > > >> prefix like we do it for all the other new igt testcases. Forcing 
> > > > >> libdrm
> > > > >> to get updated for igt all the time can get annoying fast.
> > > > >> -Daniel
> > > > >>
> > > > > In this case I'm trying to exercise new API functions in libdrm which
> > > > > wrap the GETPARAM ioctl. Would you rather me bypass the wrapper to
> > > > > avoid requiring updated libdrm? I can do that, but it fails to test 
> > > > > the
> > > > > complete path that client would use.
> > > > 
> > > > 
> > > > Am I missing something, or does 2.4.60 not exist yet?
> > > > 
> > > > That said dependency bumps for igt seem like less of an issue than
> > > > dependency bumps for mesa..  I mean if you are using igt you are
> > > > probably on the latest anyways..  I'm not sure why Daniel is so
> > > > concerned about that..
> > > > 
> > > > (but dependency bumps to something that doesn't exist yet should
> > > > perhaps be avoided)
> > > 
> > > I'd like to avoid massive depency loops for igt tests so that I can merge
> > > the testcase right when the patches land in -nightly. Otherwise there's
> > > always a small delay involved where regression can creep in. Also if I
> > > have to update libdrm every time I update igt that's annoying since
> > > without that I don't have to install/update anything at all - I run igt
> > > in-place. And we've used the LOCAL_ prefixes for pretty much every abi
> > > addition in igt tests thus far.
> > > -Daniel
> > 
> > I understand that and it certainly makes sense when libdrm is only
> > providing defines or structs. But as I said, in this case there is
> > code in libdrm (the wrapper) that we could test as part of the
> > complete path. Are you recommending that I implement duplicate
> > wrapper functions in igt with the local prefix?
> 
> Sorry I totally didn't realize that. Generally we don't have a lot of igt
> testcase for libdrm really, imo it's usually simpler to just add the
> interface to each part. Since this is such a simple one there's no need to
> have a low-level test and the libdrm test on top, but that's what I'd do
> if there's something worth testing in libdrm. Because for complex
> functionality I really want to get the bare-metal tests in together with
> the kernel part. Stalling for libdrm release could take longer.
> 
> And yes, personally I'd just have open-coded the getparam call here in
> igt, but that's just a bikeshed.
> -Daniel
> -- 

Scratch this patch. I just sent v2 of the previous patch that removes
the dependency on libdrm update.
-Jeff
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [Beignet] [PATCH 2/2 v2] Query the driver directly for compute units and subslice

[Jeff McGee]

On Thu, Mar 12, 2015 at 10:08:54AM +0800, Zhigang Gong wrote:
> LGTM,
> 
> Reviewed-by: Zhigang Gong 
> 
> Thanks.
> 

Thanks for the review, Zhigang.

With beignet portion reviewed, review should be able to proceed for
the i915, libdrm, and igt parts. These are all quite simple. Can someone(s)
please review?

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

Re: [Intel-gfx] [PATCH v4 00/11] Added missing changes for Turbo feature on SKL

[Jesse Barnes]

On 03/05/2015 09:37 PM, akash.g...@intel.com wrote:
> From: Akash Goel 
> 
> This patch series add the missing changes, required for proper
> functioning of the Turbo feature on SKL. Most of the changes are
> mainly due to the fact that on SKL, the frequency has to be programmed
> in units of 16.66 MHZ and the time period value programmed in Up/Down
> EI & threshold registers, is in units of 1.333 micro seconds.
> In this version, review comments from Chris & Ville have been addressed
> and a new patch has been added to enable the RPS interrupts programming
> for SKL also.
> 
> Akash Goel (11):
>   drm/i915/skl: Added new macros
>   drm/i915/skl: Updated intel_gpu_freq() and intel_freq_opcode()
>   drm/i915/skl: Updated the gen6_init_rps_frequencies function
>   drm/i915/skl: Updated the gen6_set_rps function
>   drm/i915/skl: Restructured the gen6_set_rps_thresholds function
>   drm/i915/skl: Updated the gen6_rps_limits function
>   drm/i915/skl: Updated the gen9_enable_rps function
>   drm/i915/skl: Updated the i915_frequency_info debugfs function
>   drm/i915/skl: Updated the act_freq_mhz_show sysfs function
>   drm/i915/skl: Enabling processing of Turbo interrupts
>   drm/i915/skl: Enable the RPS interrupts programming
> 
>  drivers/gpu/drm/i915/i915_debugfs.c |  25 --
>  drivers/gpu/drm/i915/i915_drv.h |   1 +
>  drivers/gpu/drm/i915/i915_irq.c |   5 --
>  drivers/gpu/drm/i915/i915_reg.h |   9 +++
>  drivers/gpu/drm/i915/i915_sysfs.c   |   4 +-
>  drivers/gpu/drm/i915/intel_pm.c | 147 
> ++--
>  6 files changed, 105 insertions(+), 86 deletions(-)

It looks like turbo is working, but the pm_rps test is still having a
little trouble.  I patched this on top to make it happier, but there are
still problems with the test getting to the expected frequencies, please
check out https://bugs.freedesktop.org/show_bug.cgi?id=89123.

--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6696,7 +6696,8 @@ static int chv_freq_opcode(struct drm_i915_private
*dev_pr
 int intel_gpu_freq(struct drm_i915_private *dev_priv, int val)
 {
if (IS_GEN9(dev_priv->dev))
-   return (val * GT_FREQUENCY_MULTIPLIER) / GEN9_FREQ_SCALER;
+   return DIV_ROUND_CLOSEST(val * GT_FREQUENCY_MULTIPLIER,
+   GEN9_FREQ_SCALER);
else if (IS_CHERRYVIEW(dev_priv->dev))
return chv_gpu_freq(dev_priv, val);
else if (IS_VALLEYVIEW(dev_priv->dev))
@@ -6708,7 +6709,8 @@ int intel_gpu_freq(struct drm_i915_private
*dev_priv, int
 int intel_freq_opcode(struct drm_i915_private *dev_priv, int val)
 {
if (IS_GEN9(dev_priv->dev))
-   return (val * GEN9_FREQ_SCALER) / GT_FREQUENCY_MULTIPLIER;
+   return DIV_ROUND_CLOSEST(val * GEN9_FREQ_SCALER,
+   GT_FREQUENCY_MULTIPLIER);
else if (IS_CHERRYVIEW(dev_priv->dev))
return chv_freq_opcode(dev_priv, val);
else if (IS_VALLEYVIEW(dev_priv->dev))


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

[Intel-gfx] [PATCH i-g-t v3] tests/core_getparams: Create new test core_getparams

[jeff . mcgee]

From: Jeff McGee 

New test core_getparams consists of 2 subtests, each one testing
the ability of userspace to query the correct value of a GT config
attribute: subslice total or EU total. drm/i915 implementation of
these queries is required for Cherryview and Gen9+ devices (non-
simulated).

v2: Duplicate small amount of new libdrm functionality to avoid
bumping libdrm version requirement (Daniel). Convert some
igt_asserts to the appropriate comparison variants. Add a
test description.
v3: Actually use the LOCAL GETPARAM defines. Otherwise can't build
against older libdrm as intended by v2.

For: VIZ-4636
Signed-off-by: Jeff McGee 
---
 tests/.gitignore   |   1 +
 tests/Makefile.sources |   1 +
 tests/core_getparams.c | 167 +
 3 files changed, 169 insertions(+)
 create mode 100644 tests/core_getparams.c

diff --git a/tests/.gitignore b/tests/.gitignore
index 426cc67..c742308 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -1,6 +1,7 @@
 # Please keep sorted alphabetically
 core_get_client_auth
 core_getclient
+core_getparams
 core_getstats
 core_getversion
 drm_import_export
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 51e8376..999c8f8 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -15,6 +15,7 @@ NOUVEAU_TESTS_M = \
 
 TESTS_progs_M = \
core_get_client_auth \
+   core_getparams \
drv_suspend \
drv_hangman \
gem_bad_reloc \
diff --git a/tests/core_getparams.c b/tests/core_getparams.c
new file mode 100644
index 000..2855d06
--- /dev/null
+++ b/tests/core_getparams.c
@@ -0,0 +1,167 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Jeff McGee 
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include "drmtest.h"
+#include "intel_chipset.h"
+#include "intel_bufmgr.h"
+
+IGT_TEST_DESCRIPTION("Tests the export of parameters via 
DRM_IOCTL_I915_GETPARAM\n");
+
+int drm_fd;
+int devid;
+
+static void
+init(void)
+{
+   drm_fd = drm_open_any();
+   devid = intel_get_drm_devid(drm_fd);
+}
+
+static void
+deinit(void)
+{
+   close(drm_fd);
+}
+
+#define LOCAL_I915_PARAM_SUBSLICE_TOTAL33
+#define LOCAL_I915_PARAM_EU_TOTAL  34
+
+static int
+getparam(int param, int *value)
+{
+   drm_i915_getparam_t gp;
+   int ret;
+
+   memset(&gp, 0, sizeof(gp));
+   gp.value = value;
+   gp.param = param;
+   ret = drmIoctl(drm_fd, DRM_IOCTL_I915_GETPARAM, &gp);
+   if (ret)
+   return -errno;
+
+   return 0;
+}
+
+static void
+subslice_total(void)
+{
+   unsigned int subslice_total = 0;
+   int ret;
+
+   ret = getparam(LOCAL_I915_PARAM_SUBSLICE_TOTAL, (int*)&subslice_total);
+
+   if (ret) {
+   /*
+* These devices are not required to implement the
+* interface. If they do not, -ENODEV must be returned.
+   */
+   if ((intel_gen(devid) < 8) ||
+   IS_BROADWELL(devid) ||
+   igt_run_in_simulation()) {
+   igt_assert_eq(ret, -ENODEV);
+   igt_info("subslice total: unknown\n");
+   /*
+* All other devices must implement the interface, so
+* fail them if we are here.
+   */
+   } else {
+   igt_assert_neq(ret, EINVAL); /* request not recognized? 
*/
+   igt_assert_neq(ret, ENODEV); /* device not supported? */
+   igt_assert_eq(ret, 0); /* other error? */
+   }
+   } else {
+   /*
+* On success, just make sure the returned count value is
+* non-zero. The validity of the count value for the given
+* device is not checked.
+