[Intel-gfx] [PATCH] drm: prevent double-(un)registration for connectors

2016-12-18 Thread Daniel Vetter 
If we're unlucky then the registration from a hotplugged connector
might race with the final registration step on driver load. And since
MST topology discover is asynchronous that's even somewhat likely.

v2: Also update the kerneldoc for @registered!

v3: Review from Chris:
- Improve kerneldoc for late_register/early_unregister callbacks.
- Use mutex_destroy.

Reviewed-by: Chris Wilson 
Cc: Chris Wilson 
Reported-by: Chris Wilson 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_connector.c | 20 +++-
 include/drm/drm_connector.h | 16 +++-
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b4e09b00..5906775b1fa8 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -223,6 +223,7 @@ int drm_connector_init(struct drm_device *dev,
 
INIT_LIST_HEAD(&connector->probed_modes);
INIT_LIST_HEAD(&connector->modes);
+   mutex_init(&connector->mutex);
connector->edid_blob_ptr = NULL;
connector->status = connector_status_unknown;
 
@@ -358,6 +359,8 @@ void drm_connector_cleanup(struct drm_connector *connector)
connector->funcs->atomic_destroy_state(connector,
   connector->state);
 
+   mutex_destroy(&connector->mutex);
+
memset(connector, 0, sizeof(*connector));
 }
 EXPORT_SYMBOL(drm_connector_cleanup);
@@ -373,14 +376,15 @@ EXPORT_SYMBOL(drm_connector_cleanup);
  */
 int drm_connector_register(struct drm_connector *connector)
 {
-   int ret;
+   int ret = 0;
 
+   mutex_lock(&connector->mutex);
if (connector->registered)
-   return 0;
+   goto unlock;
 
ret = drm_sysfs_connector_add(connector);
if (ret)
-   return ret;
+   goto unlock;
 
ret = drm_debugfs_connector_add(connector);
if (ret) {
@@ -396,12 +400,14 @@ int drm_connector_register(struct drm_connector 
*connector)
drm_mode_object_register(connector->dev, &connector->base);
 
connector->registered = true;
-   return 0;
+   goto unlock;
 
 err_debugfs:
drm_debugfs_connector_remove(connector);
 err_sysfs:
drm_sysfs_connector_remove(connector);
+unlock:
+   mutex_unlock(&connector->mutex);
return ret;
 }
 EXPORT_SYMBOL(drm_connector_register);
@@ -414,8 +420,11 @@ EXPORT_SYMBOL(drm_connector_register);
  */
 void drm_connector_unregister(struct drm_connector *connector)
 {
-   if (!connector->registered)
+   mutex_lock(&connector->mutex);
+   if (!connector->registered) {
+   mutex_unlock(&connector->mutex);
return;
+   }
 
if (connector->funcs->early_unregister)
connector->funcs->early_unregister(connector);
@@ -424,6 +433,7 @@ void drm_connector_unregister(struct drm_connector 
*connector)
drm_debugfs_connector_remove(connector);
 
connector->registered = false;
+   mutex_unlock(&connector->mutex);
 }
 EXPORT_SYMBOL(drm_connector_unregister);
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 0e41a2e184a9..e4f23cc2aaab 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -381,6 +381,8 @@ struct drm_connector_funcs {
 * core drm connector interfaces. Everything added from this callback
 * should be unregistered in the early_unregister callback.
 *
+* This is called while holding drm_connector->mutex.
+*
 * Returns:
 *
 * 0 on success, or a negative error code on failure.
@@ -395,6 +397,8 @@ struct drm_connector_funcs {
 * late_register(). It is called from drm_connector_unregister(),
 * early in the driver unload sequence to disable userspace access
 * before data structures are torndown.
+*
+* This is called while holding drm_connector->mutex.
 */
void (*early_unregister)(struct drm_connector *connector);
 
@@ -559,7 +563,6 @@ struct drm_cmdline_mode {
  * @interlace_allowed: can this connector handle interlaced modes?
  * @doublescan_allowed: can this connector handle doublescan?
  * @stereo_allowed: can this connector handle stereo modes?
- * @registered: is this connector exposed (registered) with userspace?
  * @modes: modes available on this connector (from fill_modes() + user)
  * @status: one of the drm_connector_status enums (connected, not, or unknown)
  * @probed_modes: list of modes derived directly from the display
@@ -608,6 +611,13 @@ struct drm_connector {
char *name;
 
/**
+* @mutex: Lock for general connector state, but currently only protects
+* @registered. Most of the connector state is still protected by the
+* mutex in &drm_mode_config.
+*/
+   struct mutex mutex;
+
+   /**
 * @index: Compacted connec

Re: [Intel-gfx] [PATCH] drm: prevent double-(un)registration for connectors

2016-12-18 Thread Daniel Vetter 
On Sun, Dec 18, 2016 at 02:35:45PM +0100, Daniel Vetter wrote:
> If we're unlucky then the registration from a hotplugged connector
> might race with the final registration step on driver load. And since
> MST topology discover is asynchronous that's even somewhat likely.
> 
> v2: Also update the kerneldoc for @registered!
> 
> v3: Review from Chris:
> - Improve kerneldoc for late_register/early_unregister callbacks.
> - Use mutex_destroy.
> 
> Reviewed-by: Chris Wilson 
> Cc: Chris Wilson 
> Reported-by: Chris Wilson 
> Signed-off-by: Daniel Vetter 

Oops, that was supposed to be in-reply-to v2.
-Daniel

> ---
>  drivers/gpu/drm/drm_connector.c | 20 +++-
>  include/drm/drm_connector.h | 16 +++-
>  2 files changed, 30 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index b4e09b00..5906775b1fa8 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -223,6 +223,7 @@ int drm_connector_init(struct drm_device *dev,
>  
>   INIT_LIST_HEAD(&connector->probed_modes);
>   INIT_LIST_HEAD(&connector->modes);
> + mutex_init(&connector->mutex);
>   connector->edid_blob_ptr = NULL;
>   connector->status = connector_status_unknown;
>  
> @@ -358,6 +359,8 @@ void drm_connector_cleanup(struct drm_connector 
> *connector)
>   connector->funcs->atomic_destroy_state(connector,
>  connector->state);
>  
> + mutex_destroy(&connector->mutex);
> +
>   memset(connector, 0, sizeof(*connector));
>  }
>  EXPORT_SYMBOL(drm_connector_cleanup);
> @@ -373,14 +376,15 @@ EXPORT_SYMBOL(drm_connector_cleanup);
>   */
>  int drm_connector_register(struct drm_connector *connector)
>  {
> - int ret;
> + int ret = 0;
>  
> + mutex_lock(&connector->mutex);
>   if (connector->registered)
> - return 0;
> + goto unlock;
>  
>   ret = drm_sysfs_connector_add(connector);
>   if (ret)
> - return ret;
> + goto unlock;
>  
>   ret = drm_debugfs_connector_add(connector);
>   if (ret) {
> @@ -396,12 +400,14 @@ int drm_connector_register(struct drm_connector 
> *connector)
>   drm_mode_object_register(connector->dev, &connector->base);
>  
>   connector->registered = true;
> - return 0;
> + goto unlock;
>  
>  err_debugfs:
>   drm_debugfs_connector_remove(connector);
>  err_sysfs:
>   drm_sysfs_connector_remove(connector);
> +unlock:
> + mutex_unlock(&connector->mutex);
>   return ret;
>  }
>  EXPORT_SYMBOL(drm_connector_register);
> @@ -414,8 +420,11 @@ EXPORT_SYMBOL(drm_connector_register);
>   */
>  void drm_connector_unregister(struct drm_connector *connector)
>  {
> - if (!connector->registered)
> + mutex_lock(&connector->mutex);
> + if (!connector->registered) {
> + mutex_unlock(&connector->mutex);
>   return;
> + }
>  
>   if (connector->funcs->early_unregister)
>   connector->funcs->early_unregister(connector);
> @@ -424,6 +433,7 @@ void drm_connector_unregister(struct drm_connector 
> *connector)
>   drm_debugfs_connector_remove(connector);
>  
>   connector->registered = false;
> + mutex_unlock(&connector->mutex);
>  }
>  EXPORT_SYMBOL(drm_connector_unregister);
>  
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 0e41a2e184a9..e4f23cc2aaab 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -381,6 +381,8 @@ struct drm_connector_funcs {
>* core drm connector interfaces. Everything added from this callback
>* should be unregistered in the early_unregister callback.
>*
> +  * This is called while holding drm_connector->mutex.
> +  *
>* Returns:
>*
>* 0 on success, or a negative error code on failure.
> @@ -395,6 +397,8 @@ struct drm_connector_funcs {
>* late_register(). It is called from drm_connector_unregister(),
>* early in the driver unload sequence to disable userspace access
>* before data structures are torndown.
> +  *
> +  * This is called while holding drm_connector->mutex.
>*/
>   void (*early_unregister)(struct drm_connector *connector);
>  
> @@ -559,7 +563,6 @@ struct drm_cmdline_mode {
>   * @interlace_allowed: can this connector handle interlaced modes?
>   * @doublescan_allowed: can this connector handle doublescan?
>   * @stereo_allowed: can this connector handle stereo modes?
> - * @registered: is this connector exposed (registered) with userspace?
>   * @modes: modes available on this connector (from fill_modes() + user)
>   * @status: one of the drm_connector_status enums (connected, not, or 
> unknown)
>   * @probed_modes: list of modes derived directly from the display
> @@ -608,6 +611,13 @@ struct drm_connector {
>   char *name;
>  
>   /**
> +  *

Re: [Intel-gfx] [PATCH 09/13] drm: Tighten locking in drm_mode_getconnector

2016-12-18 Thread Daniel Vetter 
On Fri, Dec 16, 2016 at 10:03:32AM -0500, Sean Paul wrote:
> On Tue, Dec 13, 2016 at 6:08 PM, Daniel Vetter  wrote:
> > - Modeset state needs mode_config->connection mutex, that covers
> >   figuring out the encoder, and reading properties (since in the
> >   atomic case those need to look at connector->state).
> >
> > - Don't hold any locks for stuff that's invariant (i.e. possible
> >   connectors).
> >
> > - Same for connector lookup and unref, those don't need any locks.
> >
> > - And finally the probe stuff is only protected by mode_config->mutex.
> >
> > While at it updated the kerneldoc for these fields in drm_connector
> > and add docs explaining what's protected by which locks.
> >
> 
> 
> Reviewed-by: Sean Paul 

Merged up to this patch to drm-misc, thanks for the reviews to everyone.
-Daniel

> 
> > Signed-off-by: Daniel Vetter 
> > ---
> >  drivers/gpu/drm/drm_connector.c | 92 
> > -
> >  include/drm/drm_connector.h | 23 +--
> >  2 files changed, 63 insertions(+), 52 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_connector.c 
> > b/drivers/gpu/drm/drm_connector.c
> > index 0d4728704099..44b556d5d40c 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -1160,43 +1160,65 @@ int drm_mode_getconnector(struct drm_device *dev, 
> > void *data,
> >
> > memset(&u_mode, 0, sizeof(struct drm_mode_modeinfo));
> >
> > -   mutex_lock(&dev->mode_config.mutex);
> > -
> > connector = drm_connector_lookup(dev, out_resp->connector_id);
> > -   if (!connector) {
> > -   ret = -ENOENT;
> > -   goto out_unlock;
> > -   }
> > +   if (!connector)
> > +   return -ENOENT;
> > +
> > +   drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> > +   encoder = drm_connector_get_encoder(connector);
> > +   if (encoder)
> > +   out_resp->encoder_id = encoder->base.id;
> > +   else
> > +   out_resp->encoder_id = 0;
> > +
> > +   ret = drm_mode_object_get_properties(&connector->base, 
> > file_priv->atomic,
> > +   (uint32_t __user *)(unsigned 
> > long)(out_resp->props_ptr),
> > +   (uint64_t __user *)(unsigned 
> > long)(out_resp->prop_values_ptr),
> > +   &out_resp->count_props);
> > +   drm_modeset_unlock(&dev->mode_config.connection_mutex);
> > +   if (ret)
> > +   goto out_unref;
> >
> > for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)
> > if (connector->encoder_ids[i] != 0)
> > encoders_count++;
> >
> > +   if ((out_resp->count_encoders >= encoders_count) && encoders_count) 
> > {
> > +   copied = 0;
> > +   encoder_ptr = (uint32_t __user *)(unsigned 
> > long)(out_resp->encoders_ptr);
> > +   for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> > +   if (connector->encoder_ids[i] != 0) {
> > +   if (put_user(connector->encoder_ids[i],
> > +encoder_ptr + copied)) {
> > +   ret = -EFAULT;
> > +   goto out_unref;
> > +   }
> > +   copied++;
> > +   }
> > +   }
> > +   }
> > +   out_resp->count_encoders = encoders_count;
> > +
> > +   out_resp->connector_id = connector->base.id;
> > +   out_resp->connector_type = connector->connector_type;
> > +   out_resp->connector_type_id = connector->connector_type_id;
> > +
> > +   mutex_lock(&dev->mode_config.mutex);
> > if (out_resp->count_modes == 0) {
> > connector->funcs->fill_modes(connector,
> >  dev->mode_config.max_width,
> >  dev->mode_config.max_height);
> > }
> >
> > -   /* delayed so we get modes regardless of pre-fill_modes state */
> > -   list_for_each_entry(mode, &connector->modes, head)
> > -   if (drm_mode_expose_to_userspace(mode, file_priv))
> > -   mode_count++;
> > -
> > -   out_resp->connector_id = connector->base.id;
> > -   out_resp->connector_type = connector->connector_type;
> > -   out_resp->connector_type_id = connector->connector_type_id;
> > out_resp->mm_width = connector->display_info.width_mm;
> > out_resp->mm_height = connector->display_info.height_mm;
> > out_resp->subpixel = connector->display_info.subpixel_order;
> > out_resp->connection = connector->status;
> >
> > -   drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> > -   encoder = drm_connector_get_encoder(connector);
> > -   if (encoder)
> > -   out_resp->encoder_id = encoder->base.id;
> > -   else
> >

Re: [Intel-gfx] [PATCH 0/2] drm: link status property and DP link training failure handling

2016-12-18 Thread Daniel Vetter 
On Sat, Dec 17, 2016 at 05:47:56AM +, Pandiyan, Dhinakaran wrote:
> On Fri, 2016-12-16 at 16:47 +0200, Jani Nikula wrote:
> > On Fri, 16 Dec 2016, Daniel Vetter  wrote:
> > > On Fri, Dec 16, 2016 at 12:29:05PM +0200, Jani Nikula wrote:
> > >> The two remaining patches from [1], rebased.
> > >> 
> > >> BR,
> > >> Jani.
> > >> 
> > >> 
> > >> [1] 
> > >> 1480984058-552-1-git-send-email-manasi.d.navare@intel.com">http://mid.mail-archive.com/1480984058-552-1-git-send-email-manasi.d.navare@intel.com
> > >
> > > Just for the record, I think the only thing missing here is the Xorg
> > > review on the -modesetting patch. As soon as we have that I can vacuum
> > > this up (probably best through drm-misc, but not sure).
> > 
> > Yeah I rebased this (and provided a debug hack privately) so Martin can
> > test the modesetting changes.
> > 
> > BR,
> > Jani.
> > 
> > 
> 
> I tested the -modesetting patch, which Martin had provided to Manasi,
> with a compliance testing device (DPR-120) that can simulate link
> training failure. The link rate correctly lowered after the link_status
> property was set to BAD by the kernel and the userspace responded with a
> modeset. 
> 
> One thing that was not straight forward to figure out was I had to boot
> with i915.nuclear_pageflip=1. Is it documented somewhere that the
> property needs DRIVER_ATOMIC to be set, or is it implicit?

It should work without DRIVER_ATOMIC. At least the property should be
exposed ... If this does only work with DRIVER_ATOMIC set then we have a
bug somewhere. Can you pls try to figure out why it doesn't work?

> The other thing I had trouble with -modesetting was, there was no
> modeset following a long pulse from the sink at the begging of the test.
> I had to force a modeset by changing the resolution so that the link
> training path is executed. However, the link training failure induced a
> modeset without any intervention.

Sounds roughly like how it's supposed to work. For real mode configuration
changes the desktop environment is supposed to set the mode it wants, by
listening to the xrandr hotplug event. That's not the same as the udev
hotplug event. You can listen for the xrandr hotplug event using

$ xev -event randr

Cheers, Daniel

> 
> -DK
> 
> 
> > > -Daniel
> > >
> > >> 
> > >> 
> > >> Manasi Navare (2):
> > >>   drm: Add a new connector atomic property for link status
> > >>   drm/i915: Implement Link Rate fallback on Link training failure
> > >> 
> > >>  drivers/gpu/drm/drm_atomic.c  | 16 +
> > >>  drivers/gpu/drm/drm_atomic_helper.c   | 15 
> > >>  drivers/gpu/drm/drm_connector.c   | 52 
> > >> +++
> > >>  drivers/gpu/drm/i915/intel_dp.c   | 27 ++
> > >>  drivers/gpu/drm/i915/intel_dp_link_training.c | 22 ++--
> > >>  drivers/gpu/drm/i915/intel_drv.h  |  3 ++
> > >>  include/drm/drm_connector.h   | 19 ++
> > >>  include/drm/drm_mode_config.h |  5 +++
> > >>  include/uapi/drm/drm_mode.h   |  4 +++
> > >>  9 files changed, 161 insertions(+), 2 deletions(-)
> > >> 
> > >> -- 
> > >> 2.1.4
> > >> 
> > >> ___
> > >> dri-devel mailing list
> > >> dri-de...@lists.freedesktop.org
> > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [CI 1/7] drm/i915: Add a reminder that i915_vma_move_to_active() requires struct_mutex

2016-12-18 Thread Chris Wilson 
i915_vma_move_to_active() requires the struct_mutex for serialisation
with retirement, so mark it up with lockdep_assert_held().

Signed-off-by: Chris Wilson 
Reviewed-by: Tvrtko Ursulin 
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index d665a33229bd..c64438f8171c 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1259,6 +1259,7 @@ void i915_vma_move_to_active(struct i915_vma *vma,
struct drm_i915_gem_object *obj = vma->obj;
const unsigned int idx = req->engine->id;
 
+   lockdep_assert_held(&req->i915->drm.struct_mutex);
GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
 
/* Add a reference if we're newly entering the active list.
-- 
2.11.0

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

[Intel-gfx] [CI 6/7] drm/i915/execlists: Request the kernel context be pinned high

2016-12-18 Thread Chris Wilson 
PIN_HIGH is an expensive operation (in comparison to allocating from the
hole stack) unsuitable for frequent use (such as switching between
contexts). However, the kernel context should be pinned just once for
the lifetime of the driver, and here it is appropriate to keep it out of
the mappable range (in order to maximise mappable space for users).

Signed-off-by: Chris Wilson 
Reviewed-by: Tvrtko Ursulin 
---
 drivers/gpu/drm/i915/intel_lrc.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 599afedc4494..1f8f35a94157 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -767,6 +767,7 @@ static int execlists_context_pin(struct intel_engine_cs 
*engine,
 struct i915_gem_context *ctx)
 {
struct intel_context *ce = &ctx->engine[engine->id];
+   unsigned int flags;
void *vaddr;
int ret;
 
@@ -781,8 +782,11 @@ static int execlists_context_pin(struct intel_engine_cs 
*engine,
goto err;
}
 
-   ret = i915_vma_pin(ce->state, 0, GEN8_LR_CONTEXT_ALIGN,
-  PIN_OFFSET_BIAS | GUC_WOPCM_TOP | PIN_GLOBAL);
+   flags = PIN_OFFSET_BIAS | GUC_WOPCM_TOP | PIN_GLOBAL;
+   if (ctx == ctx->i915->kernel_context)
+   flags |= PIN_HIGH;
+
+   ret = i915_vma_pin(ce->state, 0, GEN8_LR_CONTEXT_ALIGN, flags);
if (ret)
goto err;
 
-- 
2.11.0

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

[Intel-gfx] [CI 7/7] drm/i915: Swap if(enable_execlists) in i915_gem_request_alloc for a vfunc

2016-12-18 Thread Chris Wilson 
A fairly trivial move of a matching pair of routines (for preparing a
request for construction) onto an engine vfunc. The ulterior motive is
to be able to create a mock request implementation.

Signed-off-by: Chris Wilson 
Reviewed-by: Tvrtko Ursulin 
---
 drivers/gpu/drm/i915/i915_gem_request.c | 5 +
 drivers/gpu/drm/i915/intel_lrc.c| 4 +++-
 drivers/gpu/drm/i915/intel_lrc.h| 2 --
 drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +++-
 drivers/gpu/drm/i915/intel_ringbuffer.h | 3 +--
 5 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c 
b/drivers/gpu/drm/i915/i915_gem_request.c
index 475d557f2301..7427aac74923 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -622,10 +622,7 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
req->reserved_space = MIN_SPACE_FOR_ADD_REQUEST;
GEM_BUG_ON(req->reserved_space < engine->emit_breadcrumb_sz);
 
-   if (i915.enable_execlists)
-   ret = intel_logical_ring_alloc_request_extras(req);
-   else
-   ret = intel_ring_alloc_request_extras(req);
+   ret = engine->request_alloc(req);
if (ret)
goto err_ctx;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 1f8f35a94157..d322d3c11201 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -845,7 +845,7 @@ static void execlists_context_unpin(struct intel_engine_cs 
*engine,
i915_gem_context_put(ctx);
 }
 
-int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request 
*request)
+static int execlists_request_alloc(struct drm_i915_gem_request *request)
 {
struct intel_engine_cs *engine = request->engine;
struct intel_context *ce = &request->ctx->engine[engine->id];
@@ -1816,6 +1816,8 @@ logical_ring_default_vfuncs(struct intel_engine_cs 
*engine)
engine->context_pin = execlists_context_pin;
engine->context_unpin = execlists_context_unpin;
 
+   engine->request_alloc = execlists_request_alloc;
+
engine->emit_flush = gen8_emit_flush;
engine->emit_breadcrumb = gen8_emit_breadcrumb;
engine->emit_breadcrumb_sz = gen8_emit_breadcrumb_sz;
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index b5630331086a..01ba36ea125e 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -63,8 +63,6 @@ enum {
 };
 
 /* Logical Rings */
-int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request 
*request);
-int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request);
 void intel_logical_ring_stop(struct intel_engine_cs *engine);
 void intel_logical_ring_cleanup(struct intel_engine_cs *engine);
 int logical_render_ring_init(struct intel_engine_cs *engine);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 00ff572541b6..69ccf4fd22f0 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2103,7 +2103,7 @@ void intel_legacy_submission_resume(struct 
drm_i915_private *dev_priv)
}
 }
 
-int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request)
+static int ring_request_alloc(struct drm_i915_gem_request *request)
 {
int ret;
 
@@ -2598,6 +2598,8 @@ static void intel_ring_default_vfuncs(struct 
drm_i915_private *dev_priv,
engine->context_pin = intel_ring_context_pin;
engine->context_unpin = intel_ring_context_unpin;
 
+   engine->request_alloc = ring_request_alloc;
+
engine->emit_breadcrumb = i9xx_emit_breadcrumb;
engine->emit_breadcrumb_sz = i9xx_emit_breadcrumb_sz;
if (i915.semaphores) {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 4f1271821fa9..0969de7d5ab7 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -270,6 +270,7 @@ struct intel_engine_cs {
   struct i915_gem_context *ctx);
void(*context_unpin)(struct intel_engine_cs *engine,
 struct i915_gem_context *ctx);
+   int (*request_alloc)(struct drm_i915_gem_request *req);
int (*init_context)(struct drm_i915_gem_request *req);
 
int (*emit_flush)(struct drm_i915_gem_request *request,
@@ -491,8 +492,6 @@ void intel_engine_cleanup(struct intel_engine_cs *engine);
 
 void intel_legacy_submission_resume(struct drm_i915_private *dev_priv);
 
-int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request);
-
 int __must_check intel_ring_begin(struct drm_i915_gem_request *req, int n);
 int __must_check intel_ring_cacheline_align(struct drm_i915_gem_request *req);
 
-- 
2.11.0

___
Intel-gfx mailing

[Intel-gfx] [CI 5/7] drm/i915: Mark the shadow gvt context as closed

2016-12-18 Thread Chris Wilson 
As the shadow gvt is not user accessible and does not have an associated
vm, we can mark it as closed during its construction. This saves leaking
the internal knowledge of i915_gem_context into gvt/.

Signed-off-by: Chris Wilson 
Reviewed-by: Tvrtko Ursulin 
---
 drivers/gpu/drm/i915/gvt/scheduler.c| 10 +-
 drivers/gpu/drm/i915/i915_gem_context.c |  1 +
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c 
b/drivers/gpu/drm/i915/gvt/scheduler.c
index 4db242250235..fd2b026f7ecd 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -549,18 +549,10 @@ int intel_gvt_init_workload_scheduler(struct intel_gvt 
*gvt)
 
 void intel_vgpu_clean_gvt_context(struct intel_vgpu *vgpu)
 {
-   struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
-
atomic_notifier_chain_unregister(&vgpu->shadow_ctx->status_notifier,
&vgpu->shadow_ctx_notifier_block);
 
-   mutex_lock(&dev_priv->drm.struct_mutex);
-
-   /* a little hacky to mark as ctx closed */
-   vgpu->shadow_ctx->closed = true;
-   i915_gem_context_put(vgpu->shadow_ctx);
-
-   mutex_unlock(&dev_priv->drm.struct_mutex);
+   i915_gem_context_put_unlocked(vgpu->shadow_ctx);
 }
 
 int intel_vgpu_init_gvt_context(struct intel_vgpu *vgpu)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 598a70d2b695..15b25c1b1f4d 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -409,6 +409,7 @@ i915_gem_context_create_gvt(struct drm_device *dev)
if (IS_ERR(ctx))
goto out;
 
+   ctx->closed = true; /* not user accessible */
ctx->execlists_force_single_submission = true;
ctx->ring_size = 512 * PAGE_SIZE; /* Max ring buffer size */
 out:
-- 
2.11.0

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

[Intel-gfx] [CI 2/7] drm/i915: Move intel_lrc_context_pin() to avoid the forward declaration

2016-12-18 Thread Chris Wilson 
Just a simple move to avoid a forward declaration, though the diff likes
to present itself as a move of intel_logical_ring_alloc_request_extras()
in the opposite direction.

Signed-off-by: Chris Wilson 
Reviewed-by: Tvrtko Ursulin 
---
 drivers/gpu/drm/i915/intel_lrc.c | 132 +++
 1 file changed, 65 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 877adedbf833..b848b5f205ce 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -230,8 +230,6 @@ enum {
 
 static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
struct intel_engine_cs *engine);
-static int intel_lr_context_pin(struct i915_gem_context *ctx,
-   struct intel_engine_cs *engine);
 static void execlists_init_reg_state(u32 *reg_state,
 struct i915_gem_context *ctx,
 struct intel_engine_cs *engine,
@@ -774,71 +772,6 @@ static void execlists_schedule(struct drm_i915_gem_request 
*request, int prio)
/* XXX Do we need to preempt to make room for us and our deps? */
 }
 
-int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request 
*request)
-{
-   struct intel_engine_cs *engine = request->engine;
-   struct intel_context *ce = &request->ctx->engine[engine->id];
-   int ret;
-
-   /* Flush enough space to reduce the likelihood of waiting after
-* we start building the request - in which case we will just
-* have to repeat work.
-*/
-   request->reserved_space += EXECLISTS_REQUEST_SIZE;
-
-   if (!ce->state) {
-   ret = execlists_context_deferred_alloc(request->ctx, engine);
-   if (ret)
-   return ret;
-   }
-
-   request->ring = ce->ring;
-
-   ret = intel_lr_context_pin(request->ctx, engine);
-   if (ret)
-   return ret;
-
-   if (i915.enable_guc_submission) {
-   /*
-* Check that the GuC has space for the request before
-* going any further, as the i915_add_request() call
-* later on mustn't fail ...
-*/
-   ret = i915_guc_wq_reserve(request);
-   if (ret)
-   goto err_unpin;
-   }
-
-   ret = intel_ring_begin(request, 0);
-   if (ret)
-   goto err_unreserve;
-
-   if (!ce->initialised) {
-   ret = engine->init_context(request);
-   if (ret)
-   goto err_unreserve;
-
-   ce->initialised = true;
-   }
-
-   /* Note that after this point, we have committed to using
-* this request as it is being used to both track the
-* state of engine initialisation and liveness of the
-* golden renderstate above. Think twice before you try
-* to cancel/unwind this request now.
-*/
-
-   request->reserved_space -= EXECLISTS_REQUEST_SIZE;
-   return 0;
-
-err_unreserve:
-   if (i915.enable_guc_submission)
-   i915_guc_wq_unreserve(request);
-err_unpin:
-   intel_lr_context_unpin(request->ctx, engine);
-   return ret;
-}
-
 static int intel_lr_context_pin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
 {
@@ -911,6 +844,71 @@ void intel_lr_context_unpin(struct i915_gem_context *ctx,
i915_gem_context_put(ctx);
 }
 
+int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request 
*request)
+{
+   struct intel_engine_cs *engine = request->engine;
+   struct intel_context *ce = &request->ctx->engine[engine->id];
+   int ret;
+
+   /* Flush enough space to reduce the likelihood of waiting after
+* we start building the request - in which case we will just
+* have to repeat work.
+*/
+   request->reserved_space += EXECLISTS_REQUEST_SIZE;
+
+   if (!ce->state) {
+   ret = execlists_context_deferred_alloc(request->ctx, engine);
+   if (ret)
+   return ret;
+   }
+
+   request->ring = ce->ring;
+
+   ret = intel_lr_context_pin(request->ctx, engine);
+   if (ret)
+   return ret;
+
+   if (i915.enable_guc_submission) {
+   /*
+* Check that the GuC has space for the request before
+* going any further, as the i915_add_request() call
+* later on mustn't fail ...
+*/
+   ret = i915_guc_wq_reserve(request);
+   if (ret)
+   goto err_unpin;
+   }
+
+   ret = intel_ring_begin(request, 0);
+   if (ret)
+   goto err_unreserve;
+
+   if (!ce->initialised) {
+   ret = engine->init_context(request);
+   if (ret)
+

[Intel-gfx] [CI 3/7] drm/i915: Unify active context tracking between legacy/execlists/guc

2016-12-18 Thread Chris Wilson 
The requests conversion introduced a nasty bug where we could generate a
new request in the middle of constructing a request if we needed to idle
the system in order to evict space for a context. The request to idle
would be executed (and waited upon) before the current one, creating a
minor havoc in the seqno accounting, as we will consider the current
request to already be completed (prior to deferred seqno assignment) but
ring->last_retired_head would have been updated and still could allow
us to overwrite the current request before execution.

We also employed two different mechanisms to track the active context
until it was switched out. The legacy method allowed for waiting upon an
active context (it could forcibly evict any vma, including context's),
but the execlists method took a step backwards by pinning the vma for
the entire active lifespan of the context (the only way to evict was to
idle the entire GPU, not individual contexts). However, to circumvent
the tricky issue of locking (i.e. we cannot take struct_mutex at the
time of i915_gem_request_submit(), where we would want to move the
previous context onto the active tracker and unpin it), we take the
execlists approach and keep the contexts pinned until retirement.
The benefit of the execlists approach, more important for execlists than
legacy, was the reduction in work in pinning the context for each
request - as the context was kept pinned until idle, it could short
circuit the pinning for all active contexts.

We introduce new engine vfuncs to pin and unpin the context
respectively. The context is pinned at the start of the request, and
only unpinned when the following request is retired (this ensures that
the context is idle and coherent in main memory before we unpin it). We
move the engine->last_context tracking into the retirement itself
(rather than during request submission) in order to allow the submission
to be reordered or unwound without undue difficultly.

And finally an ulterior motive for unifying context handling was to
prepare for mock requests.

v2: Rename to last_retired_context, split out legacy_context tracking
for MI_SET_CONTEXT.

Signed-off-by: Chris Wilson 
Reviewed-by: Tvrtko Ursulin 
---
 drivers/gpu/drm/i915/i915_drv.h|   4 --
 drivers/gpu/drm/i915/i915_gem.c|   2 +-
 drivers/gpu/drm/i915/i915_gem_context.c| 110 +
 drivers/gpu/drm/i915/i915_gem_request.c|  38 ++
 drivers/gpu/drm/i915/i915_gem_request.h|  11 ---
 drivers/gpu/drm/i915/i915_guc_submission.c |  11 ---
 drivers/gpu/drm/i915/i915_perf.c   |  18 ++---
 drivers/gpu/drm/i915/intel_display.c   |   3 +-
 drivers/gpu/drm/i915/intel_engine_cs.c |  21 +-
 drivers/gpu/drm/i915/intel_lrc.c   |  62 ++--
 drivers/gpu/drm/i915/intel_lrc.h   |   5 +-
 drivers/gpu/drm/i915/intel_pm.c|   2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c|  58 +--
 drivers/gpu/drm/i915/intel_ringbuffer.h|  23 +-
 14 files changed, 151 insertions(+), 217 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ea9de1a15ebb..dec4ddf132f7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2446,7 +2446,6 @@ struct drm_i915_private {
struct i915_perf_stream *exclusive_stream;
 
u32 specific_ctx_id;
-   struct i915_vma *pinned_rcs_vma;
 
struct hrtimer poll_check_timer;
wait_queue_head_t poll_wq;
@@ -3488,9 +3487,6 @@ int i915_gem_context_open(struct drm_device *dev, struct 
drm_file *file);
 void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
 int i915_switch_context(struct drm_i915_gem_request *req);
 int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv);
-struct i915_vma *
-i915_gem_context_pin_legacy(struct i915_gem_context *ctx,
-   unsigned int flags);
 void i915_gem_context_free(struct kref *ctx_ref);
 struct i915_gem_context *
 i915_gem_context_create_gvt(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 782be625d37d..9b308af89ada 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4206,7 +4206,7 @@ static void assert_kernel_context_is_current(struct 
drm_i915_private *dev_priv)
enum intel_engine_id id;
 
for_each_engine(engine, dev_priv, id)
-   GEM_BUG_ON(engine->last_context != dev_priv->kernel_context);
+   GEM_BUG_ON(engine->last_retired_context != 
dev_priv->kernel_context);
 }
 
 int i915_gem_suspend(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index a57c22659a3c..598a70d2b695 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i

[Intel-gfx] [CI 4/7] drm/i915: Simplify releasing context reference

2016-12-18 Thread Chris Wilson 
A few users only take the struct_mutex in order to release a reference
to a context. We can expose a kref_put_mutex() wrapper in order to
simplify these users, and optimise taking of the mutex to the final
unref.

Signed-off-by: Chris Wilson 
Reviewed-by: Tvrtko Ursulin 
---
 drivers/gpu/drm/i915/i915_drv.h  |  7 +++
 drivers/gpu/drm/i915/i915_perf.c | 16 
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index dec4ddf132f7..6217f01d3c11 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3518,6 +3518,13 @@ static inline void i915_gem_context_put(struct 
i915_gem_context *ctx)
kref_put(&ctx->ref, i915_gem_context_free);
 }
 
+static inline void i915_gem_context_put_unlocked(struct i915_gem_context *ctx)
+{
+   kref_put_mutex(&ctx->ref,
+  i915_gem_context_free,
+  &ctx->i915->drm.struct_mutex);
+}
+
 static inline struct intel_timeline *
 i915_gem_context_lookup_timeline(struct i915_gem_context *ctx,
 struct intel_engine_cs *engine)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index da8537cb8136..a1b7eec58be2 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1555,8 +1555,6 @@ static long i915_perf_ioctl(struct file *file,
  */
 static void i915_perf_destroy_locked(struct i915_perf_stream *stream)
 {
-   struct drm_i915_private *dev_priv = stream->dev_priv;
-
if (stream->enabled)
i915_perf_disable_locked(stream);
 
@@ -1565,11 +1563,8 @@ static void i915_perf_destroy_locked(struct 
i915_perf_stream *stream)
 
list_del(&stream->link);
 
-   if (stream->ctx) {
-   mutex_lock(&dev_priv->drm.struct_mutex);
-   i915_gem_context_put(stream->ctx);
-   mutex_unlock(&dev_priv->drm.struct_mutex);
-   }
+   if (stream->ctx)
+   i915_gem_context_put_unlocked(stream->ctx);
 
kfree(stream);
 }
@@ -1738,11 +1733,8 @@ i915_perf_open_ioctl_locked(struct drm_i915_private 
*dev_priv,
 err_alloc:
kfree(stream);
 err_ctx:
-   if (specific_ctx) {
-   mutex_lock(&dev_priv->drm.struct_mutex);
-   i915_gem_context_put(specific_ctx);
-   mutex_unlock(&dev_priv->drm.struct_mutex);
-   }
+   if (specific_ctx)
+   i915_gem_context_put_unlocked(specific_ctx);
 err:
return ret;
 }
-- 
2.11.0

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

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [CI,1/7] drm/i915: Add a reminder that i915_vma_move_to_active() requires struct_mutex

2016-12-18 Thread Patchwork 
== Series Details ==

Series: series starting with [CI,1/7] drm/i915: Add a reminder that 
i915_vma_move_to_active() requires struct_mutex
URL   : https://patchwork.freedesktop.org/series/16963/
State : success

== Summary ==

Series 16963v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/16963/revisions/1/mbox/

Test drv_module_reload:
Subgroup basic-reload-inject:
dmesg-warn -> PASS   (fi-ilk-650)

fi-bdw-5557u total:247  pass:233  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050 total:247  pass:208  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205 total:247  pass:222  dwarn:0   dfail:0   fail:0   skip:25 
fi-bxt-t5700 total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-j1900 total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820 total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770  total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-ilk-650   total:247  pass:195  dwarn:0   dfail:0   fail:0   skip:52 
fi-ivb-3520m total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770  total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hqtotal:247  pass:227  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k total:247  pass:224  dwarn:3   dfail:0   fail:0   skip:20 
fi-skl-6770hqtotal:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600  total:247  pass:215  dwarn:0   dfail:0   fail:0   skip:32 

cfec01a9359f681fc9335102fb0a9bb9d3a25455 drm-tip: 2016y-12m-18d-13h-52m-40s UTC 
integration manifest
57e2731 drm/i915: Swap if(enable_execlists) in i915_gem_request_alloc for a 
vfunc
0dfdd32 drm/i915/execlists: Request the kernel context be pinned high
852aae3 drm/i915: Mark the shadow gvt context as closed
3d9ae87 drm/i915: Simplify releasing context reference
be21686 drm/i915: Unify active context tracking between legacy/execlists/guc
ea86223 drm/i915: Move intel_lrc_context_pin() to avoid the forward declaration
c6cd720 drm/i915: Add a reminder that i915_vma_move_to_active() requires 
struct_mutex

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3317/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Repeat flush of idle work during suspend

2016-12-18 Thread Chris Wilson 
The idle work handler is self-arming - if it detects that it needs to
run again it will queue itself from its work handler. Take greater care
when trying to drain the idle work, and double check that it is flushed.

The free worker has a similar issue where it is armed by an RCU task
which may be running concurrently with us.

This should hopefully help with the sporadic WARN_ON(dev_priv->gt.awake)
from i915_gem_suspend.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_gem.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9b308af89ada..7a48b264fd18 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4245,8 +4245,12 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 
cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
cancel_delayed_work_sync(&dev_priv->gt.retire_work);
-   flush_delayed_work(&dev_priv->gt.idle_work);
-   flush_work(&dev_priv->mm.free_work);
+   while (flush_delayed_work(&dev_priv->gt.idle_work))
+   ;
+
+   rcu_barrier();
+   while (flush_work(&dev_priv->mm.free_work))
+   rcu_barrier();
 
/* Assert that we sucessfully flushed all the work and
 * reset the GPU back to its idle, low power state.
-- 
2.11.0

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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Repeat flush of idle work during suspend

2016-12-18 Thread Patchwork 
== Series Details ==

Series: drm/i915: Repeat flush of idle work during suspend
URL   : https://patchwork.freedesktop.org/series/16964/
State : success

== Summary ==

Series 16964v1 drm/i915: Repeat flush of idle work during suspend
https://patchwork.freedesktop.org/api/1.0/series/16964/revisions/1/mbox/

Test drv_module_reload:
Subgroup basic-reload-inject:
dmesg-warn -> PASS   (fi-ilk-650)

fi-bdw-5557u total:247  pass:233  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050 total:247  pass:208  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205 total:247  pass:222  dwarn:0   dfail:0   fail:0   skip:25 
fi-byt-j1900 total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820 total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770  total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-ilk-650   total:247  pass:195  dwarn:0   dfail:0   fail:0   skip:52 
fi-ivb-3520m total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770  total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hqtotal:247  pass:227  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k total:247  pass:224  dwarn:3   dfail:0   fail:0   skip:20 
fi-skl-6770hqtotal:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600  total:247  pass:215  dwarn:0   dfail:0   fail:0   skip:32 

2a932d085375c80a1bbb332799db3df9738e8eba drm-tip: 2016y-12m-18d-16h-31m-05s UTC 
integration manifest
529b47f drm/i915: Repeat flush of idle work during suspend

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3318/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 3/3] drm/i915: Simplify gem stolen initialization.

2016-12-18 Thread Rodrigo Vivi 
Let's take usage of IS_LP to simplify the gem stolen
initialization as suggest by Tvrtko.

Also assume that all new LP platforms follows the chv+
and others bdw+.

Broxton and Geminilake were probably using the incorrect path
so far.

Cc: Ander Conselvan de Oliveira 
Cc: Imre Deak 
Cc: Mika Kuoppala 
Cc: Tvrtko Ursulin 
Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/i915_gem_stolen.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 3d24227..98deaa7 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -454,13 +454,12 @@ int i915_gem_init_stolen(struct drm_i915_private 
*dev_priv)
 &reserved_size);
break;
default:
-   if (IS_BROADWELL(dev_priv) ||
-   IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
-   bdw_get_stolen_reserved(dev_priv, &reserved_base,
+   if (IS_LP(dev_priv))
+   chv_get_stolen_reserved(dev_priv, &reserved_base,
&reserved_size);
else
-   chv_get_stolen_reserved(dev_priv, &reserved_base,
-&reserved_size);
+   bdw_get_stolen_reserved(dev_priv, &reserved_base,
+   &reserved_size);
break;
}
 
-- 
1.9.1

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

[Intel-gfx] [PATCH 2/3] drm/i915: Rename get stolen functions for LP platforms chv+

2016-12-18 Thread Rodrigo Vivi 
gen8 is used for both Broadwell and Cherryview but this
function here is only Cherryview and all next atom LP platforms.
So let's rename it to avoid confusion as suggested by Ville.

Cc: Ville Syrjälä 
Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/i915_gem_stolen.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index efc0e74..3d24227 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -360,7 +360,7 @@ static void gen7_get_stolen_reserved(struct 
drm_i915_private *dev_priv,
}
 }
 
-static void gen8_get_stolen_reserved(struct drm_i915_private *dev_priv,
+static void chv_get_stolen_reserved(struct drm_i915_private *dev_priv,
 unsigned long *base, unsigned long *size)
 {
uint32_t reg_val = I915_READ(GEN6_STOLEN_RESERVED);
@@ -459,7 +459,7 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
bdw_get_stolen_reserved(dev_priv, &reserved_base,
&reserved_size);
else
-   gen8_get_stolen_reserved(dev_priv, &reserved_base,
+   chv_get_stolen_reserved(dev_priv, &reserved_base,
 &reserved_size);
break;
}
-- 
1.9.1

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

[Intel-gfx] [PATCH 1/3] drm/i915: Expand is_lp backwards to gen8_lp and gen7_lp.

2016-12-18 Thread Rodrigo Vivi 
Valleyview/Baytrail (gen7_lp) and Cherryview/Braswell (gen8_lp)
are both Atom platforms like Broxton/Apollolake and Geminilake.

So let's expand this is_lp back to these platforms and
create the IS_LP(dev_priv) so we can start simplifying a bit
our if/else for platform lists.

Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/i915_drv.h | 1 +
 drivers/gpu/drm/i915/i915_pci.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6217f01..e47273c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2829,6 +2829,7 @@ static inline struct scatterlist *__sg_next(struct 
scatterlist *sg)
 #define IS_GEN9(dev_priv)  (!!((dev_priv)->info.gen_mask & BIT(8)))
 
 #define IS_GEN9_LP(dev_priv)   (IS_GEN9(dev_priv) && 
INTEL_INFO(dev_priv)->is_lp)
+#define IS_LP(dev_priv)(INTEL_INFO(dev_priv)->is_lp)
 
 #define ENGINE_MASK(id)BIT(id)
 #define RENDER_RINGENGINE_MASK(RCS)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 93f50ef..9885458 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -267,6 +267,7 @@
 
 #define VLV_FEATURES  \
.gen = 7, .num_pipes = 2, \
+   .is_lp = 1, \
.has_psr = 1, \
.has_runtime_pm = 1, \
.has_rc6 = 1, \
@@ -326,6 +327,7 @@
 static const struct intel_device_info intel_cherryview_info = {
.gen = 8, .num_pipes = 3,
.has_hotplug = 1,
+   .is_lp = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
.platform = INTEL_CHERRYVIEW,
.has_64bit_reloc = 1,
-- 
1.9.1

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

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Expand is_lp backwards to gen8_lp and gen7_lp.

2016-12-18 Thread Patchwork 
== Series Details ==

Series: series starting with [1/3] drm/i915: Expand is_lp backwards to gen8_lp 
and gen7_lp.
URL   : https://patchwork.freedesktop.org/series/16967/
State : success

== Summary ==

Series 16967v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/16967/revisions/1/mbox/


fi-bdw-5557u total:247  pass:233  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050 total:247  pass:208  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205 total:247  pass:222  dwarn:0   dfail:0   fail:0   skip:25 
fi-bxt-t5700 total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-j1900 total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820 total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770  total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-ilk-650   total:247  pass:194  dwarn:1   dfail:0   fail:0   skip:52 
fi-ivb-3520m total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770  total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hqtotal:247  pass:227  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k total:247  pass:224  dwarn:3   dfail:0   fail:0   skip:20 
fi-skl-6770hqtotal:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600  total:247  pass:215  dwarn:0   dfail:0   fail:0   skip:32 

2a932d085375c80a1bbb332799db3df9738e8eba drm-tip: 2016y-12m-18d-16h-31m-05s UTC 
integration manifest
6ab89fc drm/i915: Simplify gem stolen initialization.
3aa6bb7 drm/i915: Rename get stolen functions for LP platforms chv+
9c49d92a drm/i915: Expand is_lp backwards to gen8_lp and gen7_lp.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3319/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [Regression report] Weekly regression report WW51

2016-12-18 Thread Jairo Miramontes 

Link to FDO regression list:

https://bugs.freedesktop.org/buglist.cgi?bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&bug_status=NEEDINFO&component=DRM%2FIntel&f0=OP&f1=OP&f2=short_desc&f3=keywords&f4=CP&f5=CP&j1=OR&known_name=i915%20regressions&list_id=600614&o2=anywordssubstr&o3=anywordssubstr&op_sys=All&op_sys=Linux%20%28All%29&op_sys=other&order=bug_id%20DESC&product=DRI&query_based_on=i915%20regressions&query_format=advanced&rep_platform=All&rep_platform=x86%20%28IA32%29&rep_platform=x86-64%20%28AMD64%29&rep_platform=IA64%20%28Itanium%29&rep_platform=Other&v2=regression%20bisect&v3=regression%20bisected%20pending_bisect

Total regressions: 40

This week regressions:3
+---+--+++
| BugId | Summary | Created on | Bisect |
+---+--+++
| 99134 | REGRESSION: Intermittent hang on login kernel 
4.10-pre-rc1   | 2016-12-18 | No |
| 99113 | REGRESSION: WARN_ON_ONCE(!intel_dp->lane_count) [and slow path 
probing dp link)  | 2016-12-16 | No |
| 99093 | [PNV][regression] N450 and D510 machines get stuck in 
igt@gem_ringfill@basic-def | 2016-12-15 | No |

+---+--+++

Previous regressions:37
+---+--+++
| BugId | Summary | Created on | Bisect |
+---+--+++
| 99001 | [HSW] GPU HANG: ecode 7:0:0x85dc, in glxspheres64 [4492], 
reason: Hang on re | 2016-12-05 | Yes|
| 98919 | [Broadwell regression] Monitor on DP docking station causes 
odd behavior | 2016-12-01 | No |
| 98695 | [byt bisected] 
WARN_ON(!power_domains->domain_use_count[domain]) | 2016-11-12 | Yes|
| 98690 | [SKL bisected] System freeze when starting X using kernel 
4.9-rc1 or later   | 2016-11-11 | Yes|
| 98517 | Skylake gen6 suspend/resume video regression 
4.9 | 2016-10-31 | No |
| 98462 | [KBL] [IGT] [regression] kms_3d [drm:drm_dp_dpcd_access] Too 
many retries, givin | 2016-10-27 | No |
| 98290 | [BDW] kms_frontbuffer_tracking fbc-suspend 
fails | 2016-10-17 | No |
| 98289 | [BDW] kms_flip / vblank-vs-suspend-interruptible is 
failing  | 2016-10-17 | No |
| 98211 | i915 / drm crash when undocking from DP 
monitors | 2016-10-12 | No |
| 97918 | [bdw regression 4.8] Severe graphics regression, rainbow 
glitching and flickerin | 2016-09-25 | No |
| 97867 | [HSW][Regression] 4.6.7 and beyond causes screen flickering 
whereas 4.5.5 works  | 2016-09-19 | No |
| 97017 | [SKL GT3e guc bisected] ~10% performance drop in most 
benchmarks | 2016-07-21 | Yes|
| 96916 | Regression: screen flashes with PSR 
enabled  | 2016-07-13 | No |
| 96781 | [skl dp-mst] oops in atomic 
commit   | 2016-07-02 | Yes|
| 96645 | [regression 4.7] [BISECT]Low package c-states used only after 
forcing DPMS to of | 2016-06-22 | Yes|
| 96428 | [IVB bisected] [drm:intel_dp_aux_ch] *ERROR* dp aux hw did not 
signal timeout (h | 2016-06-07 | Yes|
| 95736 | [IVB bisected] *ERROR* uncleared fifo underrun on pipe 
A | 2016-05-24 | Yes|
| 95097 | [hdmi regression ilk] no signal to 
TV| 2016-04-24 | No |
| 94590 | [KBL] igt/kms_fbcon_fbt/psr-suspend 
regression   | 2016-03-17 | No |
| 94430 | [HSW+nvidia] regression: display becomes "disconnected" while 
suspended  | 2016-03-07 | No |
| 93782 | [i9xx TV][BISECT] vblank wait timeout on 
crtc| 2016-01-19 | Yes|
| 93486 | [HP Compaq dc7800 Small Form Factor PC][REGRESSION] 
suspend/resume failure   | 2015-12-23 | No |
| 93393 | Regression for Skylake modesetting in kernel 4.4 with 2 
Displays of different re | 2015-12-16 | No |
| 93361 | 12bpc hdmi causes wrong real refresh rate (swapbuffers return 
time)  | 2015-12-12 | Yes|
| 93263 | 945GM regression since 
4.3   | 2015-12-05 | 
No |
| 92414 | [Intel-gfx] As of kernel 4.3-rc1 system will not stay in S3 
suspend [REGRESSION] | 2015-10-10 | Yes|
| 92237 | [SNB]Horrible noise (audio) via DisplayPort 
[regression] | 2015-10-02 | No |
| 92050 | [regression]/bug introduced by commit 
[0e572fe7383a376992364914694c39aa7fe44c1d] | 2015-09-19 | Yes|
| 91974 | [bisected] unrecoverable black screen after k

Re: [Intel-gfx] [PATCH v2] dma-buf: Wait on the reservation object when sync'ing before CPU access

2016-12-18 Thread Inki Dae 


2016년 08월 16일 01:02에 Daniel Vetter 이(가) 쓴 글:
> On Mon, Aug 15, 2016 at 04:42:18PM +0100, Chris Wilson wrote:
>> Rendering operations to the dma-buf are tracked implicitly via the
>> reservation_object (dmabuf->resv). This is used to allow poll() to
>> wait upon outstanding rendering (or just query the current status of
>> rendering). The dma-buf sync ioctl allows userspace to prepare the
>> dma-buf for CPU access, which should include waiting upon rendering.
>> (Some drivers may need to do more work to ensure that the dma-buf mmap
>> is coherent as well as complete.)
>>
>> v2: Always wait upon the reservation object implicitly. We choose to do
>> it after the native handler in case it can do so more efficiently.
>>
>> Testcase: igt/prime_vgem
>> Testcase: igt/gem_concurrent_blit # *vgem*
>> Signed-off-by: Chris Wilson 
>> Cc: Sumit Semwal 
>> Cc: Daniel Vetter 
>> Cc: Eric Anholt 
>> Cc: linux-me...@vger.kernel.org
>> Cc: dri-de...@lists.freedesktop.org
>> Cc: linaro-mm-...@lists.linaro.org
>> Cc: linux-ker...@vger.kernel.org
>> ---
>>  drivers/dma-buf/dma-buf.c | 23 +++
>>  1 file changed, 23 insertions(+)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index ddaee60ae52a..cf04d249a6a4 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment 
>> *attach,
>>  }
>>  EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
>>  
>> +static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>> +  enum dma_data_direction direction)
>> +{
>> +bool write = (direction == DMA_BIDIRECTIONAL ||
>> +  direction == DMA_TO_DEVICE);
>> +struct reservation_object *resv = dmabuf->resv;
>> +long ret;
>> +
>> +/* Wait on any implicit rendering fences */
>> +ret = reservation_object_wait_timeout_rcu(resv, write, true,
>> +  MAX_SCHEDULE_TIMEOUT);
>> +if (ret < 0)
>> +return ret;
>> +
>> +return 0;
>> +}
>>  
>>  /**
>>   * dma_buf_begin_cpu_access - Must be called before accessing a dma_buf 
>> from the
>> @@ -608,6 +624,13 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>>  if (dmabuf->ops->begin_cpu_access)
>>  ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
>>  
>> +/* Ensure that all fences are waited upon - but we first allow
>> + * the native handler the chance to do so more efficiently if it
>> + * chooses. A double invocation here will be reasonably cheap no-op.
>> + */
>> +if (ret == 0)
>> +ret = __dma_buf_begin_cpu_access(dmabuf, direction);
> 
> Not sure we should wait first and the flush or the other way round. But I
> don't think it'll matter for any current dma-buf exporter, so meh.
> 

Sorry for late comment. I wonder there is no problem in case that GPU or other 
DMA device tries to access this dma buffer after dma_buf_begin_cpu_access call.
I think in this case, they - GPU or DMA devices - would make a mess of the dma 
buffer while CPU is accessing the buffer.

This patch is in mainline already so if this is real problem then I think we 
sould choose,
1. revert this patch from mainline
2. make sure to prevent other DMA devices to try to access the buffer while CPU 
is accessing the buffer.

Thanks.

> Reviewed-by: Daniel Vetter 
> 
> Sumits, can you pls pick this one up and put into drm-misc?
> -Daniel
> 
>> +
>>  return ret;
>>  }
>>  EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
>> -- 
>> 2.8.1
>>
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [kbuild-all] [PATCH] drm: Convert all helpers to drm_connector_list_iter

2016-12-18 Thread Ye Xiaolong 
On 12/16, Fengguang Wu wrote:
>Hi Daniel,
>
>On Fri, Dec 16, 2016 at 08:29:43AM +0100, Daniel Vetter wrote:
>>Hi Kbuild folks
>>
>>So yeah this doesn't apply because it's just 1 patch resent out of a
>>big patch series, in-reply-to the patch it replaces. So applying this
>>alone and telling me (and all the mailing lists) that it doesn't apply
>>isn't all that useful.
>>
>>And it shouldn't be too hard to detect this, since the fdo patchwork
>>instance does catch most of these partial resends successfully and
>>correctly, and will retest the entire patch series.
>
>Good point! CC Xiaolong. This scenario seems happen frequent enough in
>LKML to worth the efforts to add auto detect logic for.

Got it, I'll add auto detect logic to handle it.

Thanks,
Xiaolong

>
>Thanks,
>Fengguang
>
>>On Thu, Dec 15, 2016 at 11:59 PM, kbuild test robot  wrote:
>>>Hi Daniel,
>>>
>>>[auto build test ERROR on drm/drm-next]
>>>[also build test ERROR on next-20161215]
>>>[cannot apply to v4.9]
>>>[if your patch is applied to the wrong git tree, please drop us a note to 
>>>help improve the system]
>>>
>>>url:
>>>https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-Convert-all-helpers-to-drm_connector_list_iter/20161216-061508
>>>base:   git://people.freedesktop.org/~airlied/linux.git drm-next
>>>config: i386-randconfig-x003-201650 (attached as .config)
>>>compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
>>>reproduce:
>>># save the attached .config to linux build tree
>>>make ARCH=i386
>>>
>>>All error/warnings (new ones prefixed by >>):
>>>
>>>   drivers/gpu/drm/drm_crtc_helper.c: In function 
>>> 'drm_helper_encoder_in_use':
>drivers/gpu/drm/drm_crtc_helper.c:91:33: error: storage size of 
>'conn_iter' isn't known
>>> struct drm_connector_list_iter conn_iter;
>>>^
>drivers/gpu/drm/drm_crtc_helper.c:104:2: error: implicit declaration of 
>function 'drm_connector_list_iter_get' 
>[-Werror=implicit-function-declaration]
>>> drm_connector_list_iter_get(dev, &conn_iter);
>>> ^~~
>drivers/gpu/drm/drm_crtc_helper.c:105:2: error: implicit declaration of 
>function 'drm_for_each_connector_iter' 
>[-Werror=implicit-function-declaration]
>>> drm_for_each_connector_iter(connector, &conn_iter) {
>>> ^~~
>drivers/gpu/drm/drm_crtc_helper.c:105:53: error: expected ';' before '{' 
>token
>>> drm_for_each_connector_iter(connector, &conn_iter) {
>>>^
>>>   drivers/gpu/drm/drm_crtc_helper.c:91:33: warning: unused variable 
>>> 'conn_iter' [-Wunused-variable]
>>> struct drm_connector_list_iter conn_iter;
>>>^
>>>   drivers/gpu/drm/drm_crtc_helper.c: In function 'drm_crtc_helper_disable':
>>>   drivers/gpu/drm/drm_crtc_helper.c:446:34: error: storage size of 
>>> 'conn_iter' isn't known
>>>  struct drm_connector_list_iter conn_iter;
>>> ^
>>>   drivers/gpu/drm/drm_crtc_helper.c:452:54: error: expected ';' before '{' 
>>> token
>>>  drm_for_each_connector_iter(connector, &conn_iter) {
>>> ^
>>>   drivers/gpu/drm/drm_crtc_helper.c:446:34: warning: unused variable 
>>> 'conn_iter' [-Wunused-variable]
>>>  struct drm_connector_list_iter conn_iter;
>>> ^
>>>   drivers/gpu/drm/drm_crtc_helper.c: In function 
>>> 'drm_crtc_helper_set_config':
>>>   drivers/gpu/drm/drm_crtc_helper.c:521:33: error: storage size of 
>>> 'conn_iter' isn't known
>>> struct drm_connector_list_iter conn_iter;
>>>^
>drivers/gpu/drm/drm_crtc_helper.c:588:3: error: expected ';' before 
>'save_connector_encoders'
>>>  save_connector_encoders[count++] = connector->encoder;
>>>  ^~~
>drivers/gpu/drm/drm_crtc_helper.c:589:2: error: implicit declaration of 
>function 'drm_connector_list_iter_put' 
>[-Werror=implicit-function-declaration]
>>> drm_connector_list_iter_put(&conn_iter);
>>> ^~~
>>>   drivers/gpu/drm/drm_crtc_helper.c:633:53: error: expected ';' before '{' 
>>> token
>>> drm_for_each_connector_iter(connector, &conn_iter) {
>>>^
>>>   drivers/gpu/drm/drm_crtc_helper.c:675:53: error: expected ';' before '{' 
>>> token
>>> drm_for_each_connector_iter(connector, &conn_iter) {
>>>^
>drivers/gpu/drm/drm_crtc_helper.c:767:3: error: expected ';' before 
>'connector'
>>>  connector->encoder = save_connector_encoders[count++];
>>>  ^
>>>   drivers/gpu/drm/drm_crtc_helper.c:521:33: warning: unused variable 
>>> 'conn_iter' [-Wunused-variable]
>>> struct drm_connector_list_iter conn_iter

Re: [Intel-gfx] [PATCH 4/9] drm/i915/tdr: Add support for per engine reset recovery

2016-12-18 Thread Michel Thierry 



On 12/16/2016 12:45 PM, Chris Wilson wrote:

On Fri, Dec 16, 2016 at 12:20:05PM -0800, Michel Thierry wrote:

From: Arun Siluvery 

This change implements support for per-engine reset as an initial, less
intrusive hang recovery option to be attempted before falling back to the
legacy full GPU reset recovery mode if necessary. This is only supported
from Gen8 onwards.

Hangchecker determines which engines are hung and invokes error handler to
recover from it. Error handler schedules recovery for each of those engines
that are hung. The recovery procedure is as follows,
 - identifies the request that caused the hang and it is dropped
 - force engine to idle: this is done by issuing a reset request
 - reset and re-init engine
 - restart submissions to the engine

If engine reset fails then we fall back to heavy weight full gpu reset
which resets all engines and reinitiazes complete state of HW and SW.

v2: Rebase.

Cc: Chris Wilson 
Cc: Mika Kuoppala 
Signed-off-by: Tomas Elf 
Signed-off-by: Arun Siluvery 
Signed-off-by: Michel Thierry 
---
 drivers/gpu/drm/i915/i915_drv.c | 56 +++--
 drivers/gpu/drm/i915/i915_drv.h |  3 ++
 drivers/gpu/drm/i915/i915_gem.c |  2 +-
 drivers/gpu/drm/i915/intel_lrc.c| 12 
 drivers/gpu/drm/i915/intel_lrc.h|  1 +
 drivers/gpu/drm/i915/intel_uncore.c | 41 ---
 6 files changed, 108 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e5688edd62cd..a034793bc246 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1830,18 +1830,70 @@ void i915_reset(struct drm_i915_private *dev_priv)
  *
  * Reset a specific GPU engine. Useful if a hang is detected.
  * Returns zero on successful reset or otherwise an error code.
+ *
+ * Procedure is fairly simple:
+ *  - identifies the request that caused the hang and it is dropped
+ *  - force engine to idle: this is done by issuing a reset request
+ *  - reset engine
+ *  - restart submissions to the engine
  */
 int i915_reset_engine(struct intel_engine_cs *engine)


What's the serialisation between potential callers of
i915_reset_engine()?



I haven't seen simultaneous calls happening yet, would a 
reset_engine-specific mutex be enough?



 {
int ret;
struct drm_i915_private *dev_priv = engine->i915;

-   /* FIXME: replace me with engine reset sequence */
-   ret = -ENODEV;
+   /*
+* We need to first idle the engine by issuing a reset request,
+* then perform soft reset and re-initialize hw state, for all of
+* this GT power need to be awake so ensure it does throughout the
+* process
+*/
+   intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+
+   /*
+* the request that caused the hang is stuck on elsp, identify the
+* active request and drop it, adjust head to skip the offending
+* request to resume executing remaining requests in the queue.
+*/
+   i915_gem_reset_engine(engine);


Must freeze the engine and irqs first, before calling
i915_gem_reset_engine() (i.e. something like disable_engines_irq,
cancelling tasklet)


Will do.


Eeek note that the current i915_gem_reset_engine() is lacking a
spinlock.



The new mutex (for i915_reset_engine) should cover this.


+
+   ret = intel_engine_reset_begin(engine);
+   if (ret) {
+   DRM_ERROR("Failed to disable %s\n", engine->name);
+   goto error;
+   }
+
+   ret = intel_gpu_reset(dev_priv, intel_engine_flag(engine));
+   if (ret) {
+   DRM_ERROR("Failed to reset %s, ret=%d\n", engine->name, ret);
+   intel_engine_reset_cancel(engine);
+   goto error;
+   }
+
+   ret = engine->init_hw(engine);
+   if (ret)
+   goto error;

+   intel_engine_reset_cancel(engine);
+   intel_execlists_restart_submission(engine);


engine->init_hw(engine) *is* intel_execlists_restart_submission.



I'll make these changes,
Thanks.


+
+   intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
+   return 0;
+
+error:
/* use full gpu reset to recover on error */
set_bit(I915_RESET_IN_PROGRESS, &dev_priv->gpu_error.flags);

+   /* Engine reset is performed without taking struct_mutex, since it
+* failed we now fallback to full gpu reset. Wakeup any waiters
+* which should now see the reset_in_progress and release
+* struct_mutex for us to continue recovery.
+*/
+   rcu_read_lock();
+   intel_engine_wakeup(engine);
+   rcu_read_unlock();
+
+   intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
return ret;
 }



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

Re: [Intel-gfx] [PATCH i-g-t] tests: Add kms_plane_blinker

2016-12-18 Thread Maarten Lankhorst 
Op 15-12-16 om 18:26 schreef Ville Syrjälä:
> On Thu, Dec 15, 2016 at 06:42:08PM +0200, Ville Syrjälä wrote:
>> On Thu, Dec 15, 2016 at 04:41:51PM +0100, Maarten Lankhorst wrote:
>>> Op 15-12-16 om 16:36 schreef Ville Syrjälä:
 On Thu, Dec 15, 2016 at 04:28:52PM +0100, Maarten Lankhorst wrote:
> Op 15-12-16 om 16:23 schreef Ville Syrjälä:
>> On Thu, Dec 15, 2016 at 04:17:45PM +0100, Maarten Lankhorst wrote:
>>> Op 12-12-16 om 21:35 schreef ville.syrj...@linux.intel.com:
 From: Ville Syrjälä 

 Add a test to try out all the different plane enable/disable
 order permutations.

 Signed-off-by: Ville Syrjälä 
>>> Didn't look through the test, but sounds like
>>>
>>> kms_atomic_transitions.plane-*-transition-* ?
>>>
>>> Although that one tests a few more edge cases like modeset disable and 
>>> nonblocking updates..
>> I don't immediately see where it would try all the permutations, nor can
>> I see any crc_nonblock() stuff so doesn't seem like it could even spot
>> transient errors.
>>
> I haven't done any crc test there yet, but the double loop in 
> run_transition_test handles all combinations of planes.
 permutations > combinations

>>> It permutates too, I used it for some basic wm testing before. :)
>> Does it? I'm too lazy to reverse engineed it, so I just tried to run it
>> but it didn't want to run, so meh.
> Needed i915.nuclear_pageflip=1. Didn't even know we had such a knob.
That was always needed for any atomic test in i915. :)
> Anywyas, all it tells me is
> "Running test on pipe A with resolution 1920x1080 and sprite size 1920x1080 
> alpha 1"
> and similar lines, and mostly I just see a black screen or a blinking 
> backlight.
> So I can't tell what it's actually doing.
>
Well you can tell with --debug, it shows all atomic commits done.

It's a nice test for a 4k HDMI display, because in that case I had
big enough planes that having all planes enabled would cause a
underrun because the fifo size was too small. Patch 14 solves this
issue.

~Maarten

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

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Simplify gem stolen initialization.

2016-12-18 Thread Ander Conselvan De Oliveira 
On Sun, 2016-12-18 at 13:36 -0800, Rodrigo Vivi wrote:
> Let's take usage of IS_LP to simplify the gem stolen
> initialization as suggest by Tvrtko.
> 
> Also assume that all new LP platforms follows the chv+
> and others bdw+.
> 
> Broxton and Geminilake were probably using the incorrect path
> so far.

This patch doesn't change the path for bxt or glk, they will still use
chv_get_stolen_reserved(), which is correct. It does make things clearer though,
so with the above note removed, you can apply

Reviewed-by: Ander Conselvan de Oliveira 

to the whole series.

Ander

> 
> Cc: Ander Conselvan de Oliveira 
> Cc: Imre Deak 
> Cc: Mika Kuoppala 
> Cc: Tvrtko Ursulin 
> Signed-off-by: Rodrigo Vivi 
> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c
> b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 3d24227..98deaa7 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -454,13 +454,12 @@ int i915_gem_init_stolen(struct drm_i915_private
> *dev_priv)
>    &reserved_size);
>   break;
>   default:
> - if (IS_BROADWELL(dev_priv) ||
> - IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
> - bdw_get_stolen_reserved(dev_priv, &reserved_base,
> + if (IS_LP(dev_priv))
> + chv_get_stolen_reserved(dev_priv, &reserved_base,
>   &reserved_size);
>   else
> - chv_get_stolen_reserved(dev_priv, &reserved_base,
> -  &reserved_size);
> + bdw_get_stolen_reserved(dev_priv, &reserved_base,
> + &reserved_size);
>   break;
>   }
>  
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx