date:20180209

[Intel-gfx] [PATCH 2/5] drm/i915: Grab uncore.lock around enabling vblank evasion

2018-02-09 Thread Maarten Lankhorst

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/i915/i915_irq.c |  2 +-
 drivers/gpu/drm/i915/intel_drv.h|  1 +
 drivers/gpu/drm/i915/intel_sprite.c | 10 ++
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b886bd459acc..eda9543a0199 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -845,7 +845,7 @@ static u32 __intel_get_crtc_scanline_from_timestamp(struct 
intel_crtc *crtc)
 }
 
 /* I915_READ_FW, only for fast reads of display block, no need for forcewake 
etc. */
-static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
+int __intel_get_crtc_scanline(struct intel_crtc *crtc)
 {
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 468ec1e90e16..fbdbbe741b2f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1340,6 +1340,7 @@ static inline bool intel_irqs_enabled(struct 
drm_i915_private *dev_priv)
 }
 
 int intel_get_crtc_scanline(struct intel_crtc *crtc);
+int __intel_get_crtc_scanline(struct intel_crtc *crtc);
 void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
 u8 pipe_mask);
 void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index 971a1ea0db45..3a34be4fd956 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -119,6 +119,7 @@ void intel_pipe_update_start(const struct intel_crtc_state 
*new_crtc_state)
crtc->debug.max_vbl = max;
trace_i915_pipe_update_start(crtc);
 
+   spin_lock(_priv->uncore.lock);
for (;;) {
/*
 * prepare_to_wait() has a memory barrier, which guarantees
@@ -127,7 +128,7 @@ void intel_pipe_update_start(const struct intel_crtc_state 
*new_crtc_state)
 */
prepare_to_wait(wq, , TASK_UNINTERRUPTIBLE);
 
-   scanline = intel_get_crtc_scanline(crtc);
+   scanline = __intel_get_crtc_scanline(crtc);
if (scanline < min || scanline > max)
break;
 
@@ -137,11 +138,11 @@ void intel_pipe_update_start(const struct 
intel_crtc_state *new_crtc_state)
break;
}
 
-   local_irq_enable();
+   spin_unlock_irq(_priv->uncore.lock);
 
timeout = schedule_timeout(timeout);
 
-   local_irq_disable();
+   spin_lock_irq(_priv->uncore.lock);
}
 
finish_wait(wq, );
@@ -162,8 +163,9 @@ void intel_pipe_update_start(const struct intel_crtc_state 
*new_crtc_state)
 * FIXME figure out if BXT+ DSI suffers from this as well
 */
while (need_vlv_dsi_wa && scanline == vblank_start)
-   scanline = intel_get_crtc_scanline(crtc);
+   scanline = __intel_get_crtc_scanline(crtc);
 
+   spin_unlock(_priv->uncore.lock);
crtc->debug.scanline_start = scanline;
crtc->debug.start_vbl_time = ktime_get();
crtc->debug.start_vbl_count = intel_crtc_get_vblank_counter(crtc);
-- 
2.16.1

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

[Intel-gfx] [PATCH 4/5] drm/i915: Move all locking for plane updates to caller

2018-02-09 Thread Maarten Lankhorst

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/i915/i915_trace.h| 15 +++-
 drivers/gpu/drm/i915/intel_display.c | 74 ++--
 drivers/gpu/drm/i915/intel_pm.c  | 16 
 drivers/gpu/drm/i915/intel_sprite.c  | 61 +
 4 files changed, 52 insertions(+), 114 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_trace.h 
b/drivers/gpu/drm/i915/i915_trace.h
index d4a5776282ff..84bad38b20ae 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -194,9 +194,8 @@ TRACE_EVENT(vlv_fifo_size,
 
TP_fast_assign(
   __entry->pipe = crtc->pipe;
-  __entry->frame = 
crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
-   
   crtc->pipe);
-  __entry->scanline = intel_get_crtc_scanline(crtc);
+  __entry->frame = 
__intel_crtc_get_vblank_counter(crtc);
+  __entry->scanline = __intel_get_crtc_scanline(crtc);
   __entry->sprite0_start = sprite0_start;
   __entry->sprite1_start = sprite1_start;
   __entry->fifo_size = fifo_size;
@@ -226,9 +225,8 @@ TRACE_EVENT(intel_update_plane,
TP_fast_assign(
   __entry->pipe = crtc->pipe;
   __entry->name = plane->name;
-  __entry->frame = 
crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
-   
   crtc->pipe);
-  __entry->scanline = intel_get_crtc_scanline(crtc);
+  __entry->frame = 
__intel_crtc_get_vblank_counter(crtc);
+  __entry->scanline = __intel_get_crtc_scanline(crtc);
   memcpy(__entry->src, >state->src, 
sizeof(__entry->src));
   memcpy(__entry->dst, >state->dst, 
sizeof(__entry->dst));
   ),
@@ -254,9 +252,8 @@ TRACE_EVENT(intel_disable_plane,
TP_fast_assign(
   __entry->pipe = crtc->pipe;
   __entry->name = plane->name;
-  __entry->frame = 
crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
-   
   crtc->pipe);
-  __entry->scanline = intel_get_crtc_scanline(crtc);
+  __entry->frame = 
__intel_crtc_get_vblank_counter(crtc);
+  __entry->scanline = __intel_get_crtc_scanline(crtc);
   ),
 
TP_printk("pipe %c, plane %s, frame=%u, scanline=%u",
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 60ba5bb3f34c..02f91a15d2aa 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2734,14 +2734,17 @@ static void intel_plane_disable_noatomic(struct 
intel_crtc *crtc,
to_intel_crtc_state(crtc->base.state);
struct intel_plane_state *plane_state =
to_intel_plane_state(plane->base.state);
+   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 
intel_set_plane_visible(crtc_state, plane_state, false);
 
if (plane->id == PLANE_PRIMARY)
intel_pre_disable_primary_noatomic(>base);
 
+   spin_lock_irq(_priv->uncore.lock);
trace_intel_disable_plane(>base, crtc);
plane->disable_plane(plane, crtc);
+   spin_unlock_irq(_priv->uncore.lock);
 }
 
 static void
@@ -3255,7 +3258,6 @@ static void i9xx_update_plane(struct intel_plane *plane,
i915_reg_t reg = DSPCNTR(i9xx_plane);
int x = plane_state->main.x;
int y = plane_state->main.y;
-   unsigned long irqflags;
u32 dspaddr_offset;
 
linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
@@ -3265,8 +3267,6 @@ static void i9xx_update_plane(struct intel_plane *plane,
else
dspaddr_offset = linear_offset;
 
-   spin_lock_irqsave(_priv->uncore.lock, irqflags);
-
if (INTEL_GEN(dev_priv) < 4) {
/* pipesrc and dspsize control the size that is scaled from,
 * which should always be the user's requested size.
@@ -3303,8 +3303,6 @@ static void i9xx_update_plane(struct intel_plane *plane,
  dspaddr_offset);
}
POSTING_READ_FW(reg);
-
-   spin_unlock_irqrestore(_priv->uncore.lock, irqflags);
 }
 
 static void i9xx_disable_plane(struct intel_plane *plane,
@@ -3312,9 +3310,6 @@ static void i9xx_disable_plane(struct intel_plane *plane,
 {
struct drm_i915_private *dev_priv =

[Intel-gfx] [PATCH 5/5] drm/i915: Use DOUBLE_BUFFER_CTL on top of vblank evasion for GEN9+.

2018-02-09 Thread Maarten Lankhorst

This way, if somehow we wait too long there won't be much damage done..

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/i915/i915_reg.h | 3 +++
 drivers/gpu/drm/i915/intel_sprite.c | 6 ++
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e9c79b560823..6fb9239b786a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6588,6 +6588,9 @@ enum {
 #define  DIGITAL_PORTA_HOTPLUG_SHORT_DETECT(1 << 0)
 #define  DIGITAL_PORTA_HOTPLUG_LONG_DETECT (2 << 0)
 
+#define DOUBLE_BUFFER_CTL  _MMIO(0x44500)
+#define  DOUBLE_BUFFER_CTL_GLOBAL_DISABLE  (1 << 0)
+
 /* refresh rate hardware control */
 #define RR_HW_CTL   _MMIO(0x45300)
 #define  RR_HW_LOW_POWER_FRAMES_MASK0xff
diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index 094b331b522d..f0c378ecb8ae 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -169,6 +169,9 @@ void intel_pipe_update_start(const struct intel_crtc_state 
*new_crtc_state)
crtc->debug.start_vbl_time = ktime_get();
crtc->debug.start_vbl_count = __intel_crtc_get_vblank_counter(crtc);
 
+   if (INTEL_GEN(dev_priv) >= 9)
+   I915_WRITE_FW(DOUBLE_BUFFER_CTL, 
DOUBLE_BUFFER_CTL_GLOBAL_DISABLE);
+
trace_i915_pipe_update_vblank_evaded(crtc);
 }
 
@@ -191,6 +194,9 @@ void intel_pipe_update_end(struct intel_crtc_state 
*new_crtc_state)
 
trace_i915_pipe_update_end(crtc, end_vbl_count, scanline_end);
 
+   if (INTEL_GEN(dev_priv) >= 9)
+   I915_WRITE_FW(DOUBLE_BUFFER_CTL, 0);
+
/* We're still in the vblank-evade critical section, this can't race.
 * Would be slightly nice to just grab the vblank count and arm the
 * event outside of the critical section - the spinlock might spin for a
-- 
2.16.1

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

[Intel-gfx] [PATCH 1/5] drm/i915: Keep vblank irq enabled during vblank evasion.

2018-02-09 Thread Maarten Lankhorst

This is a nice preparation for grabbing the uncore lock during evasion.
Grabbing the spinlock with the lock held messes up the locking,
so it's easier to handover the reference to the event.

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/i915/intel_sprite.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index 3be22c0fcfb5..971a1ea0db45 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -109,10 +109,10 @@ void intel_pipe_update_start(const struct 
intel_crtc_state *new_crtc_state)
 
local_irq_disable();
 
-   if (min <= 0 || max <= 0)
+   if (WARN_ON(drm_crtc_vblank_get(>base)))
return;
 
-   if (WARN_ON(drm_crtc_vblank_get(>base)))
+   if (min <= 0 || max <= 0)
return;
 
crtc->debug.min_vbl = min;
@@ -146,8 +146,6 @@ void intel_pipe_update_start(const struct intel_crtc_state 
*new_crtc_state)
 
finish_wait(wq, );
 
-   drm_crtc_vblank_put(>base);
-
/*
 * On VLV/CHV DSI the scanline counter would appear to
 * increment approx. 1/3 of a scanline before start of vblank.
@@ -197,14 +195,13 @@ void intel_pipe_update_end(struct intel_crtc_state 
*new_crtc_state)
 * event outside of the critical section - the spinlock might spin for a
 * while ... */
if (new_crtc_state->base.event) {
-   WARN_ON(drm_crtc_vblank_get(>base) != 0);
-
spin_lock(>base.dev->event_lock);
drm_crtc_arm_vblank_event(>base, 
new_crtc_state->base.event);
spin_unlock(>base.dev->event_lock);
 
new_crtc_state->base.event = NULL;
-   }
+   } else
+   drm_crtc_vblank_put(>base);
 
local_irq_enable();
 
-- 
2.16.1

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

Re: [Intel-gfx] [PATCH v9 4/7] drm/i915/guc: Add support to return CNL specific reserved GuC WOPCM size

2018-02-09 Thread Joonas Lahtinen

Quoting Jackie Li (2018-02-09 01:03:52)
> CNL has its specific reserved GuC WOPCM size for RC6 and other hardware
> contexts.
> 
> This patch updates the code to return CNL specific reserved GuC WOPCM size
> for RC6 and other hardware contexts so that the GuC WOPCM size can be
> calculated correctly for CNL.
> 
> v9:
>  - Created a new patch for these changes originally made in v8 4/6 patch of
>this series (Sagar/Michal)
> 
> Cc: Sagar Arun Kamble 
> Cc: Michal Wajdeczko 
> Cc: Chris Wilson 
> Cc: Joonas Lahtinen 
> Signed-off-by: Jackie Li 



> diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.c 
> b/drivers/gpu/drm/i915/intel_guc_wopcm.c
> index 8b2ce49..d0185b0 100644
> --- a/drivers/gpu/drm/i915/intel_guc_wopcm.c
> +++ b/drivers/gpu/drm/i915/intel_guc_wopcm.c
> @@ -34,6 +34,9 @@ static inline u32 guc_wopcm_context_reserved_size(struct 
> intel_guc *guc)
> if (IS_GEN9_LP(i915))
> return BXT_GUC_WOPCM_RC6_CTX_RESERVED;
>  
> +   if (IS_GEN10(i915))
> +   return CNL_GUC_WOPCM_HW_CTX_RESERVED;
> +
> return 0;
>  }
>  

This should maybe be a if-else ladder, with the last branch applying to
any Gen10+, as it's a reasonable expectation that size is what it was in
the previous platform (or is it?). So;

if (gen9_lp)
...
else if (gen => 10)
...

Reviewed-by: Joonas Lahtinen 

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

Re: [Intel-gfx] ✗ Fi.CI.BAT: warning for ICL display initialization, selected patches (rev3)

2018-02-09 Thread Tomi Sarvela


On 02/08/18 15:34, Paulo Zanoni wrote:

Em Ter, 2018-02-06 às 19:54 +, Patchwork escreveu:

== Series Details ==

Series: ICL display initialization, selected patches (rev3)
URL   : https://patchwork.freedesktop.org/series/37668/
State : warning

== Summary ==

Series 37668v3 ICL display initialization, selected patches
https://patchwork.freedesktop.org/api/1.0/series/37668/revisions/3/mb
ox/

Test gem_sync:
 Subgroup basic-all:
 pass   -> SKIP   (fi-pnv-d510)
 Subgroup basic-each:
 pass   -> SKIP   (fi-pnv-d510)
 Subgroup basic-many-each:
 pass   -> SKIP   (fi-pnv-d510)
 Subgroup basic-store-all:
 pass   -> SKIP   (fi-pnv-d510)
 Subgroup basic-store-each:
 pass   -> SKIP   (fi-pnv-d510)


IGT-Version: 1.21-g3fd9b578 (x86_64) (Linux: 4.15.0-CI-Patchwork_7913+
x86_64)




The fact that these SKIPs don't happen in the test of rev2 and the
changes from both rev1->rev2 and rev2->rev3 are not related to pnv at
all suggests that these skips are unrelated to this series.

A small search on the other recent CI results points me to lots of
patches resulting in a SKIP -> pass for this test, with some others
doing the opposite. Perhaps we could signal this a a bug in the CI
results.


Result flipflops are handled through cibuglog which connects (test,host) 
to fd.o bug. There is no bug for PNV for this issue, it's quite rare as 
can be seen from


https://intel-gfx-ci.01.org/tree/drm-tip/fi-pnv-d510.html

(the bug seem to appear after failure on gem_ringfill@basic-default-hang)

So yes: in this case CI gives misleading result, but that can be easily 
checked (by human) from history. Work is being done towards cibuglog-ng 
that is much easier to use to detect and create bugs like this, and has 
better knowledge about previous runs.


Tomi
--
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Rename drm_i915_gem_request to i915_request

2018-02-09 Thread Chris Wilson

Quoting Joonas Lahtinen (2018-02-09 07:48:21)
> Quoting Chris Wilson (2018-02-09 01:11:34)
> > We want to de-emphasize the link between the request (dependency,
> > execution and fence tracking) from GEM and so rename the struct from
> > drm_i915_gem_request to i915_request. That is we may implement the GEM
> > user interface on top of requests, but they are an abstraction for
> > tracking execution rather than an implementation detail of GEM. (Since
> > they are not tied to HW, we keep the i915 prefix as opposed to intel.)
> 
> There are also some req -> rq renames in addition to function renames.
> 
> If we're touching this much code, would it make sense to at least
> consolidate the parameter names into "request" or "req" when touched
> here.

Never req. I always used rq in the pre-existing code as shorthand, and
request otherwise.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v9 3/7] drm/i915/guc: Implement dynamic GuC WOPCM offset and size

2018-02-09 Thread Joonas Lahtinen

Quoting Jackie Li (2018-02-09 01:03:51)
> Hardware may have specific restrictions on GuC WOPCM offset and size. On
> Gen9, the value of the GuC WOPCM size register needs to be larger than the
> value of GuC WOPCM offset register + a Gen9 specific offset (144KB) for
> reserved GuC WOPCM. Fail to enforce such a restriction on GuC WOPCM size
> will lead to GuC firmware execution failures. So we need add code to verify
> the GuC WOPCM offset and size to avoid any GuC failures. On the other hand,
> with current static GuC WOPCM offset and size values (512KB for both offset
> and size), the GuC WOPCM size verification will fail on Gen9 even if it can
> be fixed by lowering the GuC WOPCM offset by calculating its value based on
> HuC firmware size (which is likely less than 200KB on Gen9), so that we can
> have a GuC WOPCM size value which is large enough to pass the GuC WOPCM
> size check.
> 
> This patch updates the reserved GuC WOPCM size for RC6 context on Gen9 to
> 24KB to strictly align with the Gen9 GuC WOPCM layout and add support to
> return CNL specific hardware reserved GuC WOPCM size. It also adds support
> to verify the GuC WOPCM size aganist the Gen9 hardware restrictions.
> Meanwhile, it provides a common way to calculate GuC WOPCM offset and size
> based on GuC and HuC firmware sizes for all GuC/HuC enabled platforms.
> Currently, GuC WOPCM offset is calculated based on HuC firmware size +
> reserved WOPCM size while GuC WOPCM size is set to total WOPCM size - GuC
> WOPCM offset - hardware reserved GuC WOPCM size. In this case, GuC WOPCM
> offset will be updated based on the size of HuC firmware while GuC WOPCM
> size will be set to use all the remaining WOPCM space.
> 
> v2:
>  - Removed intel_wopcm_init (Ville/Sagar/Joonas)
>  - Renamed and Moved the intel_wopcm_partition into intel_guc (Sagar)
>  - Removed unnecessary function calls (Joonas)
>  - Init GuC WOPCM partition as soon as firmware fetching is completed
> 
> v3:
>  - Fixed indentation issues (Chris)
>  - Removed layering violation code (Chris/Michal)
>  - Created separat files for GuC wopcm code  (Michal)
>  - Used inline function to avoid code duplication (Michal)
> 
> v4:
>  - Preset the GuC WOPCM top during early GuC init (Chris)
>  - Fail intel_uc_init_hw() as soon as GuC WOPCM partitioning failed
> 
> v5:
>  - Moved GuC DMA WOPCM register updating code into intel_guc_wopcm.c
>  - Took care of the locking status before writing to GuC DMA
>Write-Once registers. (Joonas)
> 
> v6:
>  - Made sure the GuC WOPCM size to be multiple of 4K (4K aligned)
> 
> v8:
>  - Updated comments and fixed naming issues (Sagar/Joonas)
>  - Updated commit message to include more description about the hardware
>restriction on GuC WOPCM size (Sagar)
> 
> v9:
>  - Minor changes variable names and code comments (Sagar)
>  - Added detailed GuC WOPCM layout drawing (Sagar/Michal)
>  - Refined macro definitions to be reader friendly (Michal)
>  - Removed redundent check to valid flag (Michal)
>  - Unified first parameter for exported GuC WOPCM functions (Michal)
>  - Refined the name and parameter list of hardware restriction checking
>functions (Michal)
> 
> Cc: Michal Wajdeczko 
> Cc: Sagar Arun Kamble 
> Cc: Sujaritha Sundaresan 
> Cc: Daniele Ceraolo Spurio 
> Cc: John Spotswood 
> Cc: Oscar Mateo 
> Cc: Chris Wilson 
> Cc: Joonas Lahtinen 
> Reviewed-by: Sagar Arun Kamble  (v8)
> Signed-off-by: Jackie Li 



> --- a/drivers/gpu/drm/i915/intel_guc_wopcm.c
> +++ b/drivers/gpu/drm/i915/intel_guc_wopcm.c
> @@ -27,22 +27,115 @@
>  #include "intel_guc_wopcm.h"
>  #include "i915_drv.h"
>  
> +static inline u32 guc_wopcm_context_reserved_size(struct intel_guc *guc)

This is intel_guc_wopcm file, so context_reserved_size() would be enough. But if
you feel like it is needed in the usage context, it's ok this way too.

> +int intel_guc_wopcm_init(struct intel_guc_wopcm *guc_wopcm, u32 guc_fw_size,
> +u32 huc_fw_size)
>  {
> -   struct drm_i915_private *i915 = guc_to_i915(guc);
> -   u32 size = GUC_WOPCM_TOP;
> +   struct intel_guc *guc =
> +   container_of(guc_wopcm, struct intel_guc, wopcm);
> +   u32 reserved = guc_wopcm_context_reserved_size(guc);
> +   u32 offset, size, top;
> +   int err;
>  
> -   /* On BXT, the top of WOPCM is reserved for RC6 context */
> -   if (IS_GEN9_LP(i915))
> -   size -= BXT_GUC_WOPCM_RC6_RESERVED;
> +   if (!guc_fw_size)
> +   return -EINVAL;
> +
> +   if (reserved >= WOPCM_DEFAULT_SIZE)
> +   return -E2BIG;
> +
> +   offset = huc_fw_size + WOPCM_RESERVED_SIZE;
> +   if (offset >= WOPCM_DEFAULT_SIZE)
> +   return -E2BIG;

[Intel-gfx] [PATCH v7] drm/i915/userptr: Probe vma range before gup

2018-02-09 Thread Chris Wilson

We want to exclude any GGTT objects from being present on our internal
lists to avoid the deadlock we may run into with our requirement for
struct_mutex during invalidate. However, if the gup_fast fails, we put
the userptr onto the workqueue and mark it as active, so that we
remember to serialise the worker upon mmu_invalidate.

v2: Hold mmap_sem to prevent modifications to the mm while we probe and
add ourselves to the interval-tree for notificiation.
v3: Rely on mmap_sem for a simpler patch.
v4: Mark up the mmap_sem nesting
v5: Don't deactivate on -EAGAIN as that means the worker is queued
v6: Fight the indentation and chained if-else error handling
v7: Fight again.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104209
Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: Michał Winiarski 
---
 drivers/gpu/drm/i915/i915_gem_userptr.c | 162 ++--
 1 file changed, 110 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c 
b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 1f9d24021cbb..a504746230d1 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -411,7 +411,7 @@ struct get_pages_work {
struct task_struct *task;
 };
 
-static struct sg_table *
+static int
 __i915_gem_userptr_alloc_pages(struct drm_i915_gem_object *obj,
   struct page **pvec, int num_pages)
 {
@@ -422,7 +422,7 @@ __i915_gem_userptr_alloc_pages(struct drm_i915_gem_object 
*obj,
 
st = kmalloc(sizeof(*st), GFP_KERNEL);
if (!st)
-   return ERR_PTR(-ENOMEM);
+   return -ENOMEM;
 
 alloc_table:
ret = __sg_alloc_table_from_pages(st, pvec, num_pages,
@@ -431,7 +431,7 @@ __i915_gem_userptr_alloc_pages(struct drm_i915_gem_object 
*obj,
  GFP_KERNEL);
if (ret) {
kfree(st);
-   return ERR_PTR(ret);
+   return ret;
}
 
ret = i915_gem_gtt_prepare_pages(obj, st);
@@ -444,14 +444,14 @@ __i915_gem_userptr_alloc_pages(struct drm_i915_gem_object 
*obj,
}
 
kfree(st);
-   return ERR_PTR(ret);
+   return ret;
}
 
sg_page_sizes = i915_sg_page_sizes(st->sgl);
 
__i915_gem_object_set_pages(obj, st, sg_page_sizes);
 
-   return st;
+   return 0;
 }
 
 static int
@@ -532,19 +532,14 @@ __i915_gem_userptr_get_pages_worker(struct work_struct 
*_work)
 
mutex_lock(>mm.lock);
if (obj->userptr.work == >work) {
-   struct sg_table *pages = ERR_PTR(ret);
-
if (pinned == npages) {
-   pages = __i915_gem_userptr_alloc_pages(obj, pvec,
-  npages);
-   if (!IS_ERR(pages)) {
+   ret = __i915_gem_userptr_alloc_pages(obj, pvec, npages);
+   if (!ret)
pinned = 0;
-   pages = NULL;
-   }
}
 
-   obj->userptr.work = ERR_CAST(pages);
-   if (IS_ERR(pages))
+   obj->userptr.work = ERR_PTR(ret);
+   if (ret)
__i915_gem_userptr_set_active(obj, false);
}
mutex_unlock(>mm.lock);
@@ -557,7 +552,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct 
*_work)
kfree(work);
 }
 
-static struct sg_table *
+static int
 __i915_gem_userptr_get_pages_schedule(struct drm_i915_gem_object *obj)
 {
struct get_pages_work *work;
@@ -583,7 +578,7 @@ __i915_gem_userptr_get_pages_schedule(struct 
drm_i915_gem_object *obj)
 */
work = kmalloc(sizeof(*work), GFP_KERNEL);
if (work == NULL)
-   return ERR_PTR(-ENOMEM);
+   return -ENOMEM;
 
obj->userptr.work = >work;
 
@@ -595,19 +590,91 @@ __i915_gem_userptr_get_pages_schedule(struct 
drm_i915_gem_object *obj)
INIT_WORK(>work, __i915_gem_userptr_get_pages_worker);
queue_work(to_i915(obj->base.dev)->mm.userptr_wq, >work);
 
-   return ERR_PTR(-EAGAIN);
+   return -EAGAIN;
 }
 
-static int i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj)
+static int
+probe_range(struct mm_struct *mm, unsigned long addr, unsigned long len)
+{
+   const unsigned long end = addr + len;
+   struct vm_area_struct *vma;
+   int ret = -EFAULT;
+
+   for (vma = find_vma(mm, addr); vma; vma = vma->vm_next) {
+   if (vma->vm_start > addr)
+   break;
+
+   /*
+* Exclude any VMA that is backed only by struct_page, i.e.
+* IO regions that include our own GGTT mmaps. We cannot handle
+* such ranges, as we may encounter deadlocks around our
+

Re: [Intel-gfx] [RFC] drm/i915: Use DOUBLE_BUFFER_CTL instead of vblank evasion for GEN9+.

2018-02-09 Thread Maarten Lankhorst

Op 08-02-18 om 19:27 schreef Ville Syrjälä:
> On Thu, Feb 08, 2018 at 09:55:38AM +0100, Maarten Lankhorst wrote:
>> References: https://bugs.freedesktop.org/show_bug.cgi?id=104975
>> Signed-off-by: Maarten Lankhorst 
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h  |  2 ++
>>  drivers/gpu/drm/i915/i915_reg.h  |  3 +++
>>  drivers/gpu/drm/i915/intel_display.c |  1 +
>>  drivers/gpu/drm/i915/intel_sprite.c  | 16 
>>  4 files changed, 22 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index d5e695da2fc4..11251e0107f4 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1967,6 +1967,8 @@ struct drm_i915_private {
>>  /* Display functions */
>>  struct drm_i915_display_funcs display;
>>  
>> +spinlock_t display_evasion_lock;
>> +
>>  /* PCH chipset type */
>>  enum intel_pch pch_type;
>>  unsigned short pch_id;
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index 93f6ec2ea8f2..20af56644844 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6594,6 +6594,9 @@ enum {
>>  #define  DIGITAL_PORTA_HOTPLUG_SHORT_DETECT (1 << 0)
>>  #define  DIGITAL_PORTA_HOTPLUG_LONG_DETECT  (2 << 0)
>>  
>> +#define DOUBLE_BUFFER_CTL   _MMIO(0x44500)
>> +#define  DOUBLE_BUFFER_CTL_GLOBAL_DISABLE   (1 << 0)
>> +
>>  /* refresh rate hardware control */
>>  #define RR_HW_CTL   _MMIO(0x45300)
>>  #define  RR_HW_LOW_POWER_FRAMES_MASK0xff
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 707406d1bf57..81087722bc49 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -14476,6 +14476,7 @@ int intel_modeset_init(struct drm_device *dev)
>>  dev_priv->modeset_wq = alloc_ordered_workqueue("i915_modeset", 0);
>>  
>>  drm_mode_config_init(dev);
>> +spin_lock_init(_priv->display_evasion_lock);
>>  
>>  dev->mode_config.min_width = 0;
>>  dev->mode_config.min_height = 0;
>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
>> b/drivers/gpu/drm/i915/intel_sprite.c
>> index ac0a4f9c1954..4d1e9b166d57 100644
>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>> @@ -98,6 +98,13 @@ void intel_pipe_update_start(const struct 
>> intel_crtc_state *new_crtc_state)
>>  intel_crtc_has_type(new_crtc_state, INTEL_OUTPUT_DSI);
>>  DEFINE_WAIT(wait);
>>  
>> +if (INTEL_GEN(dev_priv) >= 9) {
>> +spin_lock_irq(_priv->display_evasion_lock);
>> +I915_WRITE(DOUBLE_BUFFER_CTL, DOUBLE_BUFFER_CTL_GLOBAL_DISABLE);
>> +scanline = intel_get_crtc_scanline(crtc);
>> +goto done;
>> +}
> I think we still want to do the vblank evasion. It gives us more accurate
> infromation on which frame the operation will complete. If we don't do
> it we may end up signalling the completion one frame late. Although I
> suppose it doesn't matter too much from the user POV since in each case
> we'd end up dropping a frame. Maybe for av sync etc. it might matter
> in some cases.
>
> It would become even more important if we allowed flips to be overridden
> because then we'd really need to know whether the previous flip happened
> at all or not (so that we could signal fences and whatnot correctly to
> keep userspace informed on which framebuffer is actually being scanned
> out). And we might end up holding the lock across every vblank start,
> thus preventing the display from updating at all.
>
> So I think we should use DOUBLE_BUFFER_CTL just make missing the
> deadline bit less dangerous in the presence of fragile hardware. I do
> think we should still try to optimize plane/pipe updates more to reduce
> the chances of that happening in general.
>
> Also IIRC there are some "(dis)allow double buffer disable" bits in various
> hardware blocks which have to set correctly for this to work. Did you check
> whether we're setting them appropriately?
The global switch doesn't mention this at least. It only mentions that
async MMIO and command flips are not affected.

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

Re: [Intel-gfx] [PATCH v9 2/7] drm/i915/guc: Rename guc_ggtt_offset to intel_guc_ggtt_offset

2018-02-09 Thread Joonas Lahtinen

Quoting Jackie Li (2018-02-09 01:03:50)
> GuC related exported functions should start with "intel_guc_" prefix and
> pass intel_guc as the first parameter since its guc related. Current
> guc_ggtt_offset() failed to follow this code convention and this is a
> problem for future patches that needs to access intel_guc data to verify
> the GGTT offset against the GuC WOPCM top.
> 
> This patch renames the guc_ggtt_offset to intel_guc_ggtt_offset and updates
> the related code to pass intel_guc pointer to this function call, so that
> we can have a unified coding style for GuC code and also enable the future
> patches to get GuC related data from intel_guc to do the offset
> verification. Meanwhile, this patch also moves the GUC_GGTT_TOP from
> intel_guc_regs.h to intel_guc.h since it is not GuC register related
> definition.
> 
> v8:
>  - Fixed coding style issues and moved GUC_GGTT_TOP to intel_guc.h (Sagar)
>  - Updated commit message to explain to reason and motivation to add
>intel_guc as the first parameter of intel_guc_ggtt_offset (Chris)
> 
> v9:
>  - Fixed code alignment issue due to line break (Chris)
> 
> Cc: Michal Wajdeczko 
> Cc: Sagar Arun Kamble 
> Cc: Chris Wilson 
> Cc: Joonas Lahtinen 
> Reviewed-by: Sagar Arun Kamble  (v8)
> Signed-off-by: Jackie Li 



> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -269,8 +269,10 @@ void intel_guc_init_params(struct intel_guc *guc)
>  
> /* If GuC submission is enabled, set up additional parameters here */
> if (USES_GUC_SUBMISSION(dev_priv)) {
> -   u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
> -   u32 pgs = guc_ggtt_offset(dev_priv->guc.stage_desc_pool);
> +   u32 ads = intel_guc_ggtt_offset(guc,
> +   guc->ads_vma) >> PAGE_SHIFT;
> +   u32 pgs = intel_guc_ggtt_offset(guc,
> +   
> dev_priv->guc.stage_desc_pool);

I must wonder why the other is addressed through dev_priv->guc and other
directly. guc->stage_desc_pool would work equally, right?

> @@ -135,11 +124,23 @@ int intel_guc_ads_create(struct intel_guc *guc)
> blob->ads.eng_state_size[engine->guc_id] =
> engine->context_size - skipped_size;
>  
> -   base = guc_ggtt_offset(vma);
> +   base = intel_guc_ggtt_offset(guc, vma);
> blob->ads.scheduler_policies = base + ptr_offset(blob, policies);
> blob->ads.reg_state_buffer = base + ptr_offset(blob, 
> reg_state_buffer);
> blob->ads.reg_state_addr = base + ptr_offset(blob, reg_state);
>  
> +   /*
> +* The GuC requires a "Golden Context" when it reinitialises
> +* engines after a reset. Here we use the Render ring default
> +* context, which must already exist and be pinned in the GGTT,
> +* so its address won't change after we've told the GuC where
> +* to find it. Note that we have to skip our header (1 page),
> +* because our GuC shared data is there.
> +*/
> +   vma = dev_priv->kernel_context->engine[RCS].state;

vma variable is being reused here, you want to have another variable for
each different use, this avoids nasty merge conflicts and at worst, bugs.

> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c 
> b/drivers/gpu/drm/i915/intel_guc_log.c
> index 7b5074e..eca8725 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -638,7 +638,7 @@ int intel_guc_log_create(struct intel_guc *guc)
> (GUC_LOG_ISR_PAGES << GUC_LOG_ISR_SHIFT) |
> (GUC_LOG_CRASH_PAGES << GUC_LOG_CRASH_SHIFT);
>  
> -   offset = guc_ggtt_offset(vma) >> PAGE_SHIFT; /* in pages */
> +   offset = intel_guc_ggtt_offset(guc, vma) >> PAGE_SHIFT; /* in pages */

It's obvious it's in pages, you can drop the comment.

With those addressed, this is:

Reviewed-by: Joonas Lahtinen 

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

[Intel-gfx] [PATCH 2/5] drm/i915: Grab uncore.lock around enabling vblank evasion

[Intel-gfx] [PATCH 4/5] drm/i915: Move all locking for plane updates to caller

[Intel-gfx] [PATCH 5/5] drm/i915: Use DOUBLE_BUFFER_CTL on top of vblank evasion for GEN9+.

[Intel-gfx] [PATCH 1/5] drm/i915: Keep vblank irq enabled during vblank evasion.

Re: [Intel-gfx] [PATCH v9 4/7] drm/i915/guc: Add support to return CNL specific reserved GuC WOPCM size

Re: [Intel-gfx] ✗ Fi.CI.BAT: warning for ICL display initialization, selected patches (rev3)

Re: [Intel-gfx] [PATCH] drm/i915: Rename drm_i915_gem_request to i915_request

Re: [Intel-gfx] [PATCH v9 3/7] drm/i915/guc: Implement dynamic GuC WOPCM offset and size

[Intel-gfx] [PATCH v7] drm/i915/userptr: Probe vma range before gup

Re: [Intel-gfx] [RFC] drm/i915: Use DOUBLE_BUFFER_CTL instead of vblank evasion for GEN9+.

Re: [Intel-gfx] [PATCH v9 2/7] drm/i915/guc: Rename guc_ggtt_offset to intel_guc_ggtt_offset

< 1 2

101 - 111 of 111 matches

Site Navigation

Mail list logo

Footer information