Re: [Intel-gfx] [PATCH v8 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6

[Imre Deak]

On la, 2016-02-06 at 00:13 +0530, Sagar Arun Kamble wrote:
> RC6 setup is shared between BIOS and Driver. BIOS sets up subset of
> RC6
> setup registers. If those are not setup Driver should not enable RC6.
> For implementing this, driver can check RC_CTRL0 and RC_CTRL1 values
> to know if BIOS has enabled HW/SW RC6.
> This will also enable user to control RC6 using BIOS settings alone.
> RC6 related instability can be avoided by disabling via BIOS settings
> till driver fixes it.
> 
> v2: Had placed logic in gen8 function by mistake. Fixed it.
> Ensuring RPM is not enabled in case BIOS disabled RC6.
> 
> v3: Need to disable RPM if RC6 is disabled due to BIOS settings.
> (Daniel)
> Runtime PM enabling happens before gen9_enable_rc6.
> Moved the updation of enable_rc6 parameter in intel_uncore_sanitize.
> 
> v4: Added elaborate check for BIOS RC6 setup. Prepared check_pctx for
> bxt.
> (Imre)
> 
> v5: Caching reserved stolen base and size in the driver private data.
> Reorganized RC6 setup check. Moved from gen9_enable_rc6 to
> intel_uncore_sanitize. (Imre)
> 
> v6: Rebasing on the patch submitted by Imre that moves
> gem_init_stolen
> earlier in the load.
> 
> v7: Removed PWRCTX_MAXCNT_VCSUNIT1 check as it applies to SKL. (Imre)
> 
> v8: Fixed formatting and checkpatch issues. Fixed functional issue
> where
> RC6 ctx size check was missing. (Imre)
> 
> Cc: Imre Deak 
> Signed-off-by: Sagar Arun Kamble 

Reviewed-by: Imre Deak 

Thanks for the patch, I pushed it to -dinq.

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.h|  2 ++
>  drivers/gpu/drm/i915/i915_gem_stolen.c |  3 ++
>  drivers/gpu/drm/i915/i915_reg.h| 11 +++
>  drivers/gpu/drm/i915/intel_drv.h   |  1 +
>  drivers/gpu/drm/i915/intel_pm.c| 53
> --
>  drivers/gpu/drm/i915/intel_uncore.c|  2 ++
>  6 files changed, 70 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
> b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index f520c90..66a6da2 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -342,6 +342,8 @@ struct i915_gtt {
>  
>   size_t stolen_size; /* Total size of stolen
> memory */
>   size_t stolen_usable_size;  /* Total size minus BIOS
> reserved */
> + size_t stolen_reserved_base;
> + size_t stolen_reserved_size;
>   u64 mappable_end;   /* End offset that we can
> CPU map */
>   struct io_mapping *mappable;/* Mapping to our CPU
> mappable region */
>   phys_addr_t mappable_base;  /* PA of our GMADR */
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c
> b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index c384dc9..ba1a00d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -458,6 +458,9 @@ int i915_gem_init_stolen(struct drm_device *dev)
>   return 0;
>   }
>  
> + dev_priv->gtt.stolen_reserved_base = reserved_base;
> + dev_priv->gtt.stolen_reserved_size = reserved_size;
> +
>   /* It is possible for the reserved area to end before the
> end of stolen
>    * memory, so just consider the start. */
>   reserved_total = stolen_top - reserved_base;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index c0bd691..71b1fe9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6784,6 +6784,16 @@ enum skl_disp_power_wells {
>  
>  #define  VLV_PMWGICZ _MMIO(0x1300a4)
>  
> +#define  RC6_LOCATION_MMIO(0xD40)
> +#define     RC6_CTX_IN_DRAM  (1 << 0)
> +#define  RC6_CTX_BASE_MMIO(0xD48)
> +#defineRC6_CTX_BASE_MASK 0xFFF0
> +#define  PWRCTX_MAXCNT_RCSUNIT   _MMIO(0x2054)
> +#define  PWRCTX_MAXCNT_VCSUNIT0  _MMIO(0x12054
> )
> +#define  PWRCTX_MAXCNT_BCSUNIT   _MMIO(0x22054)
> +#define  PWRCTX_MAXCNT_VECSUNIT  _MMIO(0x1A054
> )
> +#define  PWRCTX_MAXCNT_VCSUNIT1  _MMIO(0x1C054
> )
> +#defineIDLE_TIME_MASK0xF
>  #define  FORCEWAKE   _MMIO(0xA18C)
>  #define  FORCEWAKE_VLV   _MMIO(0x1300b0
> )
>  #define  FORCEWAKE_ACK_VLV   _MMIO(0x1300b4)
> @@ -6922,6 +6932,7 @@ enum skl_disp_power_wells {
>  #define GEN6_RPDEUC  _MMIO(0xA084)
>  #define GEN6_RPDEUCSW_MMIO(0xA088)
>  #define GEN6_RC_STATE_MMIO(0xA094)
> +#define   RC6_STATE  (1 << 18)
>  #define GEN6_RC1_WAKE_RATE_LIMIT _MMIO(0xA098)
>  #define GEN6_RC6_WAKE_RATE_LIMIT _MMIO(0xA09C)
>  #define 

Re: [Intel-gfx] [PATCH] drm/i915: Protect fbdev across slow or failed initialisation

[Lukas Wunner]

Hi Chris,

On Fri, Feb 05, 2016 at 11:09:27AM +, Chris Wilson wrote:
> On Fri, Feb 05, 2016 at 01:27:10AM +0100, Lukas Wunner wrote:
> > On Thu, Feb 04, 2016 at 09:21:17AM +, Li, Weinan Z wrote:
> > > We still need this patch. Seems 54632abe8ca3 ("drm/i915: Fix oops caused 
> > > by fbdev initialization
> > > failure") as well as 366e39b4d2c5 ("drm/i915: Tear down fbdev if
> > > initialization fails") this 2 patches can???t cover this case. It???s 
> > > access ifbdev->fb before the initialization
> > > finished, but not initialization failed. If don???t have any other 
> > > patches or code update relative, it may still have in 4.5.
> > 
> > Okay, I see.
> > 
> > > 
> > > add info NULL check should be better, it is also initialized in the async 
> > > queue
> > > >   info = ifbdev->helper.fbdev;
> > > > + if (info == NULL)
> > > > +return false;
> > > >   if (!info->screen_base)
> > 
> > So if there's indeed a race between fbdev initialization and the call to
> > intel_fbdev_restore_mode() (on lastclose), there's more that can go awry:
> > - intel_fbdev_restore_mode() dereferences ifbdev->fb->obj
> > - it calls __drm_atomic_helper_set_config() which WARNs if ifbdev->fb is 
> > NULL
> > - it calls drm_fb_helper_set_par() which dereferences ifbdev->helper.fbdev
> > 
> > Instead of checking all that it's probably simpler to call
> > async_synchronize_full() at the beginning of intel_fbdev_restore_mode(),
> > as Li Weinan suggested. I'll submit the corresponding one-liner patch
> > tomorrow if noone else does.
> > 
> > Chris' patch also modified intel_fbdev_set_suspend() as well as
> > intel_fbdev_output_poll_changed(), not sure if these are racy as well.
> > At least the stack traces posted by Li Weinan and Gustav Fägerlind
> > only indicate that lastclose is racy.
> 
> We called set-suspend internally, but we don't do any checks before
> hand. I was concerned we may be able to get into a situation where
> .fb_probe fails and leaves a dangling dev_priv->ifbdev for which we
> would then trip over the NULL info->screen_base. So I was looking for a
> suitable guard.

Yes, looking at this with a fresh pair of eyeballs I think you were
totally right, i915_pm_ops is declared as part of i915_pci_driver and
it might indeed happen that i915_pm_suspend() is called before the fbdev
is fully initialized.

As for intel_fbdev_output_poll_changed(), there's even a comment in
i915_load_modeset_init() stating that hotplug events might occur
during fdbev initial config.

Below patch adds the requisite async_synchronize_full() to the three
functions that you also modified and updates that comment.

However AFAICT a corner case remains where we're still vulnerable to races:
async_schedule() runs synchronously "if we're out of memory or if there's
too much work pending already" (see __async_schedule() in kernel/async.c).
In this case no entry is added to the pending list and
async_synchronize_full() might theoretically return before the synchronously
executed function has finished.

The existing call to async_synchronize_full() in intel_fbdev_fini()
seems to be susceptible to the same race condition, but it's probably
very hard to trigger it. (Unload the module before the fbdev is fully
initialized.)

To make it bullet proof we could declare a completion in intel_fbdev.c
and wait for that instead. Opinions?

Thanks,

Lukas

-- >8 --

Subject: [PATCH] drm/i915: Fix races on fbdev

The ->lastclose callback invokes intel_fbdev_restore_mode() and has
been witnessed to run before intel_fbdev_initial_config_async()
has finished.

We might likewise receive hotplug events or be suspended before
we've had a chance to fully set up the fbdev.

Fix by waiting for the asynchronous thread to finish.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93580
Reported-by: Gustav FÀgerlind 
Reported-by: "Li, Weinan Z" 
Cc: Chris Wilson 
Cc: sta...@vger.kernel.org
Signed-off-by: Lukas Wunner 
---
 drivers/gpu/drm/i915/i915_dma.c| 8 +++-
 drivers/gpu/drm/i915/intel_fbdev.c | 4 
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index a0f5659..a76b528 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -437,11 +437,9 @@ static int i915_load_modeset_init(struct drm_device *dev)
 * Some ports require correctly set-up hpd registers for detection to
 * work properly (leading to ghost connected connector status), e.g. VGA
 * on gm45.  Hence we can only set up the initial fbdev config after hpd
-* irqs are fully enabled. Now we should scan for the initial config
-* only once hotplug handling is enabled, but due to screwed-up locking
-* around kms/fbdev init we can't protect the fdbev initial config
-* scanning against hotplug events. Hence do this 

[Intel-gfx] [PATCH v5 07/11] drm/i915: tidy initialisation failure paths (legacy, part 3)

[Dave Gordon]

intel_cleanup_ring_buffer() contains one low-level register access,
which is not really appropriate for its level of abstraction.

It calls intel_stop_ring_buffer() which then calls stop_ring() -- which
is the level that deals with h/w registers -- then reads a GEN-specific
register to see whether the ring is actually now idle. It only prints a
WARNING, though, and doesn't actually refrain from continuing even if
the test fails!

So, let's move the register-level check and WARNING down into the
low-level function that's already doing register access. If we wanted
to, we could pass a pass/fail status back, but since the high-level
code continues anyway, there's no reason to at present.

As a bonus, apart from fixing the lavering violation, moving the code
lets us eliminate the implicitly-used local 'dev_priv' from the caller.

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

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 284da10..b47d140 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -551,6 +551,8 @@ static bool stop_ring(struct intel_engine_cs *ring)
I915_WRITE_MODE(ring, _MASKED_BIT_DISABLE(STOP_RING));
}
 
+   WARN_ON(!IS_GEN2(ring->dev) && (I915_READ_MODE(ring) & MODE_IDLE) == 0);
+
return (I915_READ_HEAD(ring) & HEAD_ADDR) == 0;
 }
 
@@ -2260,17 +2262,11 @@ static int intel_init_ring_buffer(struct drm_device 
*dev,
 
 void intel_cleanup_ring_buffer(struct intel_engine_cs *ring)
 {
-   struct drm_i915_private *dev_priv;
-
if (!intel_ring_initialized(ring))
return;
 
-   dev_priv = to_i915(ring->dev);
-
if (ring->buffer) {
intel_stop_ring_buffer(ring);
-   WARN_ON(!IS_GEN2(ring->dev) && (I915_READ_MODE(ring) & 
MODE_IDLE) == 0);
-
intel_unpin_ringbuffer_obj(ring->buffer);
intel_ringbuffer_free(ring->buffer);
ring->buffer = NULL;
-- 
1.9.1

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

[Intel-gfx] [PATCH v5 08/11] drm/i915: two small moves towards more consistency

[Dave Gordon]

From: Nick Hoath 

Rename i915_gem_cleanup_ringbuffer() to i915_gem_cleanup_engines(),
to relect what it actually does (clean up all the engines, not a
single ringbuffer). The name is probably a legacy from the days
when there was only one "ring" (i.e. engine) and one ringbuffer!

While we're looking at this function, there's a strangely-formatted
comment block that looks ugly, so let's prettify it.

Signed-off-by: Nick Hoath 
Signed-off-by: David Gordon 
Cc: Mika Kuoppala 
Cc: Chris Wilson 
Signed-off-by: Dave Gordon 
---
 drivers/gpu/drm/i915/i915_dma.c |  4 ++--
 drivers/gpu/drm/i915/i915_drv.h |  2 +-
 drivers/gpu/drm/i915/i915_gem.c | 23 ---
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index a42eb58..5d95c80 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -444,7 +444,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
 
 cleanup_gem:
mutex_lock(>struct_mutex);
-   i915_gem_cleanup_ringbuffer(dev);
+   i915_gem_cleanup_engines(dev);
i915_gem_context_fini(dev);
mutex_unlock(>struct_mutex);
 cleanup_irq:
@@ -1253,7 +1253,7 @@ int i915_driver_unload(struct drm_device *dev)
 
intel_guc_ucode_fini(dev);
mutex_lock(>struct_mutex);
-   i915_gem_cleanup_ringbuffer(dev);
+   i915_gem_cleanup_engines(dev);
i915_gem_context_fini(dev);
mutex_unlock(>struct_mutex);
intel_fbc_cleanup_cfb(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8216665..e11eef1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3058,7 +3058,7 @@ int i915_gem_init_rings(struct drm_device *dev);
 int __must_check i915_gem_init_hw(struct drm_device *dev);
 int i915_gem_l3_remap(struct drm_i915_gem_request *req, int slice);
 void i915_gem_init_swizzling(struct drm_device *dev);
-void i915_gem_cleanup_ringbuffer(struct drm_device *dev);
+void i915_gem_cleanup_engines(struct drm_device *dev);
 int __must_check i915_gpu_idle(struct drm_device *dev);
 int __must_check i915_gem_suspend(struct drm_device *dev);
 void __i915_add_request(struct drm_i915_gem_request *req,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2bd5b5f..c85029b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4913,7 +4913,7 @@ int i915_gem_init_rings(struct drm_device *dev)
req = i915_gem_request_alloc(ring, NULL);
if (IS_ERR(req)) {
ret = PTR_ERR(req);
-   i915_gem_cleanup_ringbuffer(dev);
+   i915_gem_cleanup_engines(dev);
goto out;
}
 
@@ -4926,7 +4926,7 @@ int i915_gem_init_rings(struct drm_device *dev)
if (ret && ret != -EIO) {
DRM_ERROR("PPGTT enable ring #%d failed %d\n", i, ret);
i915_gem_request_cancel(req);
-   i915_gem_cleanup_ringbuffer(dev);
+   i915_gem_cleanup_engines(dev);
goto out;
}
 
@@ -4934,7 +4934,7 @@ int i915_gem_init_rings(struct drm_device *dev)
if (ret && ret != -EIO) {
DRM_ERROR("Context enable ring #%d failed %d\n", i, 
ret);
i915_gem_request_cancel(req);
-   i915_gem_cleanup_ringbuffer(dev);
+   i915_gem_cleanup_engines(dev);
goto out;
}
 
@@ -5012,7 +5012,7 @@ int i915_gem_init(struct drm_device *dev)
 }
 
 void
-i915_gem_cleanup_ringbuffer(struct drm_device *dev)
+i915_gem_cleanup_engines(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *ring;
@@ -5021,13 +5021,14 @@ int i915_gem_init(struct drm_device *dev)
for_each_ring(ring, dev_priv, i)
dev_priv->gt.cleanup_ring(ring);
 
-if (i915.enable_execlists)
-/*
- * Neither the BIOS, ourselves or any other kernel
- * expects the system to be in execlists mode on startup,
- * so we need to reset the GPU back to legacy mode.
- */
-intel_gpu_reset(dev);
+   if (i915.enable_execlists) {
+   /*
+* Neither the BIOS, ourselves or any other kernel
+* expects the system to be in execlists mode on startup,
+* so we need to reset the GPU back to legacy mode.
+*/
+   intel_gpu_reset(dev);
+   }
 }
 
 static void
-- 
1.9.1

___
Intel-gfx mailing list

[Intel-gfx] [PATCH v5 00/11] A collection of cleanups, revisited

[Dave Gordon]

Various things can go wrong during initialisation and teardown, but they
usually don't, so the error-handling paths go largely untested.  This
collection of patches fixes some things I recently noticed. Some might
lead to a kernel OOPS, but mostly they're leaks and other inconsistencies.

Includes Nick Hoath's patch to swap context/engine teardown, because that
helps make setup & teardown more consistent too; but more importantly, that
patch [10/11] is dependent on patch [9/11] of this set to work correctly.
Applying [10/11] without at least [9/11] as well may cause a fault on
driver unload!

v2:
Patches reordered and the previous [1/4] split into [4..6/6],
so that the bugs that it's fixing are more clearly visible
(Mika Kuoppala, although he said it wasn't actually *necessary*).

v3:
Patches reordered again; the bug fixes are now in patches 3 and 5.
The former could stand alone; the latter depends on patch 4 and is
a prerequisite for Nick's patch [6/6] to function.

v4:
Rebased

v5:
Patches repartitioned, reordered, and rebased again.
The first two are now the ones that have already been reviewed
(R-b: Chris Wilson), the remaining ones have been broken up
into smaller more digestible chunks.

Dave Gordon (9):
  drm/i915: unmap the correct page in intel_logical_ring_cleanup()
  drm/i915: consolidate LRC mode HWSP setup & teardown
  drm/i915: remove useless copypasted code
  drm/i915: tidy up initialisation failure paths (GEM)
  drm/i915: tidy initialisation failure paths (legacy, part 1)
  drm/i915: tidy initialisation failure paths (legacy, part 2)
  drm/i915: tidy initialisation failure paths (legacy, part 3)
  drm/i915: tear down hardware status page mappings earlier
  drm/i915: make LRC status page teardown code a bit more robust

Nick Hoath (2):
  drm/i915: two small moves towards more consistency
  drm/i915: interchange context/engine cleanup order

 drivers/gpu/drm/i915/i915_dma.c |  4 +-
 drivers/gpu/drm/i915/i915_drv.h |  2 +-
 drivers/gpu/drm/i915/i915_gem.c | 28 --
 drivers/gpu/drm/i915/intel_lrc.c| 65 -
 drivers/gpu/drm/i915/intel_ringbuffer.c | 40 
 5 files changed, 75 insertions(+), 64 deletions(-)

-- 
1.9.1

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

[Intel-gfx] [PATCH v5 03/11] drm/i915: remove useless copypasted code

[Dave Gordon]

There's some unreachable code in intel_logical_ring_cleanup(),
presumably copypasted from the legacy ringbuffer version at
creation. Let's delete it :)

Signed-off-by: Dave Gordon 
---
 drivers/gpu/drm/i915/intel_lrc.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 148d291..ff38e57 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1991,17 +1991,11 @@ static int gen8_init_rcs_context(struct 
drm_i915_gem_request *req)
  */
 void intel_logical_ring_cleanup(struct intel_engine_cs *ring)
 {
-   struct drm_i915_private *dev_priv;
-
if (!intel_ring_initialized(ring))
return;
 
-   dev_priv = ring->dev->dev_private;
-
-   if (ring->buffer) {
-   intel_logical_ring_stop(ring);
-   WARN_ON((I915_READ_MODE(ring) & MODE_IDLE) == 0);
-   }
+   /* should not be set in LRC mode */
+   WARN_ON(ring->buffer);
 
if (ring->cleanup)
ring->cleanup(ring);
-- 
1.9.1

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

[Intel-gfx] [PATCH v5 04/11] drm/i915: tidy up initialisation failure paths (GEM)

[Dave Gordon]

Add call to i915_gem_context_fini() to deallocate the default context
if the call to init_rings() fails, so that we don't leak the allocated
memory in that situation.

Signed-off-by: Dave Gordon 
---
 drivers/gpu/drm/i915/i915_gem.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e9b19bc..2bd5b5f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4987,8 +4987,11 @@ int i915_gem_init(struct drm_device *dev)
goto out_unlock;
 
ret = dev_priv->gt.init_rings(dev);
-   if (ret)
+   if (ret) {
+   i915_gem_context_fini(dev);
+   /* XXX: anything else to be undone here? */
goto out_unlock;
+   }
 
ret = i915_gem_init_hw(dev);
if (ret == -EIO) {
-- 
1.9.1

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

[Intel-gfx] [PATCH v5 06/11] drm/i915: tidy initialisation failure paths (legacy, part 2)

[Dave Gordon]

After the last patch, there is only one caller of the trivial function
intel_destroy_ringbuffer_obj(), so we might as well fold it into the
caller.

v3:
   Don't bother to clear a pointer in an object about to be freed.
   [Chris Wilson]

v4:
   Don't bother to check for NULL pointer, as drm_gem_object_unreference
   can handle NULL [Chris Wilson]. This relies on 'base' being the very
   first member of 'struct intel_ringbuffer'!

Signed-off-by: Dave Gordon 
Cc: Chris Wilson 
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 3e1aec8..284da10 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2135,12 +2135,6 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device 
*dev,
return 0;
 }
 
-static void intel_destroy_ringbuffer_obj(struct intel_ringbuffer *ringbuf)
-{
-   drm_gem_object_unreference(>obj->base);
-   ringbuf->obj = NULL;
-}
-
 static int intel_alloc_ringbuffer_obj(struct drm_device *dev,
  struct intel_ringbuffer *ringbuf)
 {
@@ -2203,11 +2197,11 @@ struct intel_ringbuffer *
 }
 
 void
-intel_ringbuffer_free(struct intel_ringbuffer *ring)
+intel_ringbuffer_free(struct intel_ringbuffer *ringbuf)
 {
-   intel_destroy_ringbuffer_obj(ring);
-   list_del(>link);
-   kfree(ring);
+   drm_gem_object_unreference(>obj->base);
+   list_del(>link);
+   kfree(ringbuf);
 }
 
 static int intel_init_ring_buffer(struct drm_device *dev,
-- 
1.9.1

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

[Intel-gfx] [PATCH v5 11/11] drm/i915: make LRC status page teardown code a bit more robust

[Dave Gordon]

Having recently fixed a few ordering/lifetime bugs in this area, it
seems worthwhile putting a few more checks and warnings here, to provide
early detection of any future code changes that cause problems;
especially as we hope to rework the pinning/refcounting of the kernel
context soon.

Specifically, we want to check that teardown_status_page is called
*before* the underlying storage is freed, and that by the time
intel_logical_ring_cleanup() is called, the status page mapping have
already been released. We'll also take care to undo the status page
kmapping only if it has been set up.

Signed-off-by: Dave Gordon 
Cc: Mika Kuoppala 
Cc: Chris Wilson 
---
 drivers/gpu/drm/i915/intel_lrc.c | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 95ba8ec..b4deaca 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2003,6 +2003,9 @@ void intel_logical_ring_cleanup(struct intel_engine_cs 
*ring)
i915_cmd_parser_fini_ring(ring);
i915_gem_batch_pool_fini(>batch_pool);
 
+   /* Status page should have gone already */
+   WARN_ON(ring->status_page.page_addr);
+   WARN_ON(ring->status_page.obj);
 
ring->disable_lite_restore_wa = false;
ring->ctx_desc_template = 0;
@@ -2446,6 +2449,10 @@ void intel_lr_context_free(struct intel_context *ctx)
continue;
 
if (ctx == ctx->i915->kernel_context) {
+   /*
+* The HWSP is part of the kernel context
+* object in LRC mode, so tear it down *now*
+*/
lrc_teardown_hardware_status_page(ringbuf->ring);
intel_unpin_ringbuffer_obj(ringbuf);
i915_gem_object_ggtt_unpin(ctx_obj);
@@ -2517,12 +2524,19 @@ static void lrc_setup_hardware_status_page(struct 
intel_engine_cs *ring)
POSTING_READ(RING_HWS_PGA(ring->mmio_base));
 }
 
+/* This should be called *before* the default context is destroyed */
 static void lrc_teardown_hardware_status_page(struct intel_engine_cs *ring)
 {
-   if (ring->status_page.obj) {
+   struct drm_i915_gem_object *dctx_obj = ring->status_page.obj;
+
+   WARN_ON(!dctx_obj);
+
+   if (ring->status_page.page_addr) {
kunmap(kmap_to_page(ring->status_page.page_addr));
-   ring->status_page.obj = NULL;
+   ring->status_page.page_addr = NULL;
}
+
+   ring->status_page.obj = NULL;
 }
 
 /**
-- 
1.9.1

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

[Intel-gfx] [PATCH v5 10/11] drm/i915: interchange context/engine cleanup order

[Dave Gordon]

From: Nick Hoath 

Swap the order of context & engine cleanup, so that contexts are cleaned
up first, and *then* engines. This is a more sensible order anyway, but
in particular has become necessary since the 'intel_ring_initialized()
must be simple and inline' patch, which now uses ring->dev as an
'initialised' flag, so it can now be NULL after engine teardown. This in
turn can cause a problem in the context code, which (used to) check the
ring->dev->struct_mutex -- causing a fault if ring->dev was NULL.

Signed-off-by: Nick Hoath 
Signed-off-by: David Gordon 
Signed-off-by: Dave Gordon 
---
 drivers/gpu/drm/i915/i915_dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 5d95c80..57afec8 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -444,8 +444,8 @@ static int i915_load_modeset_init(struct drm_device *dev)
 
 cleanup_gem:
mutex_lock(>struct_mutex);
-   i915_gem_cleanup_engines(dev);
i915_gem_context_fini(dev);
+   i915_gem_cleanup_engines(dev);
mutex_unlock(>struct_mutex);
 cleanup_irq:
intel_guc_ucode_fini(dev);
@@ -1253,8 +1253,8 @@ int i915_driver_unload(struct drm_device *dev)
 
intel_guc_ucode_fini(dev);
mutex_lock(>struct_mutex);
-   i915_gem_cleanup_engines(dev);
i915_gem_context_fini(dev);
+   i915_gem_cleanup_engines(dev);
mutex_unlock(>struct_mutex);
intel_fbc_cleanup_cfb(dev_priv);
 
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH] list-workarounds: Extend the script to Mesa

[Kibey, Sameer]

> -Original Message-
> From: Lespiau, Damien
> Sent: Friday, February 05, 2016 4:16 AM
> To: Kibey, Sameer
> Cc: intel-gfx@lists.freedesktop.org; mesa-...@lists.freedesktop.org; Sharp,
> Sarah A; Widawsky, Benjamin
> Subject: Re: [PATCH] list-workarounds: Extend the script to Mesa
> 
> On Thu, Feb 04, 2016 at 06:14:02PM +, Kibey, Sameer wrote:
> > Updated the list-workarounds script so that it can parse Mesa
> > directory if provided. Moved the common code to a separate function to
> > allow reuse for both kernel and mesa.
> >
> > The new command line is:
> > Usage: list-workarounds [options] path-to-kernel
> >-k path-to-kernel -m path-to-mesa
> >
> > The legacy usage is retained to avoid breaking backwards
> > compatibility. New parameters -k and -m are added for the new
> > behavior.
> >
> > Either kernel or mesa or both paths can be specified.
> > If path-to-mesa is invalid, error is reported.
> >
> > Signed-off-by: Sameer Kibey 
> 
> Out of curiosity, how did you send the email? It doesn't seem to have been
> sent with git send-email and so the patch isn't picked up by our patchwork
> instance.

I sent the email manually, but will use git send-email next time. 

> Out of the comments below, I guess the only serious one is allowing both
> byt/vlv, but maybe mesa only uses one of the two? I wouldn't mind landing
> the patch with that answered.

I will replace byt with vlv to keep it consistent. 

> > ---
> >  scripts/list-workarounds | 75
> > ++--
> >  1 file changed, 54 insertions(+), 21 deletions(-)
> >
> > diff --git a/scripts/list-workarounds b/scripts/list-workarounds index
> > d11b6a9..0b63541 100755
> > --- a/scripts/list-workarounds
> > +++ b/scripts/list-workarounds
> > @@ -18,7 +18,7 @@ def find_nth(haystack, needle, n):
> > return start
> >
> >  valid_platforms = ('ctg', 'elk', 'ilk', 'snb', 'ivb', 'vlv', 'hsw', 'bdw',
> > -  'chv', 'skl', 'bxt')
> > +  'chv', 'skl', 'bxt', 'kbl', 'byt')
> 
> Do we really need both byt and vlv? that creates two different names for the
> same platform, which sounds like a recipe to have the actual set of W/As for
> this platform be the union of vlv and byt ones.

Agree, will remove byt. 

> >  def parse_platforms(line, p):
> > l =  p.split(',')
> > for p in l:
> > @@ -65,9 +65,15 @@ def execute(cmd):
> > return out, err
> >
> >  def parse_options(args):
> > -   usage = "Usage: list-workarounds [options] path-to-kernel"
> > +   usage = "Usage: list-workarounds [options] path-to-kernel -k path-
> to-kernel -m path-to-mesa"
> > parser = optparse.OptionParser(usage, version=1.0)
> 
> Quite frankly, I'd just remove the old behaviour.

Originally I had removed the old behavior. Ben suggested keeping it in case 
some people have it in other scripts. 

> > +   parser.add_option("-k", "--kernel-path", dest="kernel_path",
> default=None,
> > + help="path to kernel")
> > +
> > +   parser.add_option("-m", "--mesa-path", dest="mesa_path",
> default=None,
> > + help="path to mesa")
> > +
> > parser.add_option("-v", "--verbose", action="store_true",
> >   dest="verbose", default=False,
> >   help="be more verbose")
> > @@ -76,30 +82,14 @@ def parse_options(args):
> >   help="List workarounds for the specified platform")
> >
> > (options, args) = parser.parse_args()
> > -
> > return (options, args)
> >
> > -if __name__ == '__main__':
> > -   (options, args) = parse_options(sys.argv[1:])
> > -   verbose = options.verbose
> > -
> > -   if not len(args):
> > -   sys.stderr.write("error: A path to a kernel tree is
> required\n")
> > -   sys.exit(1)
> > -
> > -   kernel_path = args[0]
> > -   kconfig = os.path.join(kernel_path, 'Kconfig')
> > -   if not os.path.isfile(kconfig):
> > -   sys.stderr.write("error: %s does not point to a kernel tree \n"
> > -% kernel_path)
> > -   sys.exit(1)
> > -
> > -   i915_dir = os.path.join('drivers', 'gpu', 'drm', 'i915')
> > +def print_workarounds(code_path, driver_dir):
> > olddir = os.getcwd()
> > -   os.chdir(kernel_path)
> > +   os.chdir(code_path)
> 
> project_root?

Will change to project_root

> > work_arounds, err = execute(['git', 'grep', '-n',
> >  '-e', 'W[aA][A-Z0-9][a-zA-Z0-9_]\+',
> > -i915_dir])
> > +driver_dir])
> > os.chdir(olddir)
> > if err:
> > print(err)
> > @@ -111,3 +101,46 @@ if __name__ == '__main__':
> > print("%s: %s" % (wa, ', '.join(workarounds[wa])))
> > elif options.platform in workarounds[wa]:
> > print(wa)
> > +
> > +
> > +if __name__ == '__main__':
> > +   (options, args) = parse_options(sys.argv)
> > +   verbose = options.verbose
> > +   

[Intel-gfx] [PATCH v5 09/11] drm/i915: tear down hardware status page mappings earlier

[Dave Gordon]

This has to be done before the containing object is freed, otherwise
the mapping ends up pointing to no-longer-allocated memory :(

Signed-off-by: Dave Gordon 
Cc: Mika Kuoppala 
Cc: Chris Wilson 
---
 drivers/gpu/drm/i915/intel_lrc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index ff38e57..95ba8ec 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2003,7 +2003,6 @@ void intel_logical_ring_cleanup(struct intel_engine_cs 
*ring)
i915_cmd_parser_fini_ring(ring);
i915_gem_batch_pool_fini(>batch_pool);
 
-   lrc_teardown_hardware_status_page(ring);
 
ring->disable_lite_restore_wa = false;
ring->ctx_desc_template = 0;
@@ -2447,6 +2446,7 @@ void intel_lr_context_free(struct intel_context *ctx)
continue;
 
if (ctx == ctx->i915->kernel_context) {
+   lrc_teardown_hardware_status_page(ringbuf->ring);
intel_unpin_ringbuffer_obj(ringbuf);
i915_gem_object_ggtt_unpin(ctx_obj);
}
-- 
1.9.1

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

[Intel-gfx] [PATCH v5 01/11] drm/i915: unmap the correct page in intel_logical_ring_cleanup()

[Dave Gordon]

The kunmap() call here didn't match the corresponding kmap().
The kmap()ing was changed with the introduction of the GuC-compatible
layout of context objects and the introduction of "LRC_PPHWSP_PN", in

d167519 drm/i915: Integrate GuC-based command submission

but the corresponding kunmap() wasn't, probably because the old code
dug into the underlying sgl implementation instead of than calling
"i915_gem_object_get_page(ring->status_page.obj, 0)", which might more
easily have been noticed as containing an assumption about page 0.

v3:
Use kmap_to_page() rather than repeat the mapping calculation.
[Chris Wilson]

Signed-off-by: Dave Gordon 
Reviewed-by: Chris Wilson 
---
 drivers/gpu/drm/i915/intel_lrc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 3a03646..c551c97 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2012,7 +2012,7 @@ void intel_logical_ring_cleanup(struct intel_engine_cs 
*ring)
i915_gem_batch_pool_fini(>batch_pool);
 
if (ring->status_page.obj) {
-   kunmap(sg_page(ring->status_page.obj->pages->sgl));
+   kunmap(kmap_to_page(ring->status_page.page_addr));
ring->status_page.obj = NULL;
}
 
-- 
1.9.1

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

[Intel-gfx] [PATCH v5 05/11] drm/i915: tidy initialisation failure paths (legacy, part 1)

[Dave Gordon]

Make sure intel_unpin_ringbuffer_obj() can handle the case where the
ringbuffer has been allocated but map-and-pin failed. Return early if
the ringbuffer isn't mapped [Chris Wilson].

That allows us to simplify the cleanup path in intel_init_ring_buffer(),
as it can just take the error exit and let intel_cleanup_ring_buffer()
deal with any partially-set-up state.

Signed-off-by: Dave Gordon 
Cc: Chris Wilson 
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 6f5b511..3e1aec8 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2055,6 +2055,9 @@ static int init_phys_status_page(struct intel_engine_cs 
*ring)
 
 void intel_unpin_ringbuffer_obj(struct intel_ringbuffer *ringbuf)
 {
+   if (!ringbuf->virtual_start)
+   return;
+
if (HAS_LLC(ringbuf->obj->base.dev) && !ringbuf->obj->stolen)
vunmap(ringbuf->virtual_start);
else
@@ -2232,6 +2235,13 @@ static int intel_init_ring_buffer(struct drm_device *dev,
}
ring->buffer = ringbuf;
 
+   ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf);
+   if (ret) {
+   DRM_ERROR("Failed to pin and map ringbuffer %s: %d\n",
+   ring->name, ret);
+   goto error;
+   }
+
if (I915_NEED_GFX_HWS(dev)) {
ret = init_status_page(ring);
if (ret)
@@ -2243,14 +2253,6 @@ static int intel_init_ring_buffer(struct drm_device *dev,
goto error;
}
 
-   ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf);
-   if (ret) {
-   DRM_ERROR("Failed to pin and map ringbuffer %s: %d\n",
-   ring->name, ret);
-   intel_destroy_ringbuffer_obj(ringbuf);
-   goto error;
-   }
-
ret = i915_cmd_parser_init_ring(ring);
if (ret)
goto error;
-- 
1.9.1

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

[Intel-gfx] [PATCH v5 02/11] drm/i915: consolidate LRC mode HWSP setup & teardown

[Dave Gordon]

In legacy ringbuffer mode, the HWSP is a separate GEM object with its
own pinning and reference counts. In LRC mode, however, it's not;
instead its part of the default context object. The LRC-mode setup &
teardown code therefore needs to handle this specially; the presence
of the two bugs fixed in this patchset suggests that this code is not
well-understood or maintained at present.

So, this patch:
moves the (newly-fixed!) LRC-mode HWSP teardown code to its own
(trivial) function lrc_teardown_hardware_status_page(), and
changes the call signature of lrc_setup_hardware_status_page()
to match
so that all knowledge of this special arrangement is local to these
two functions.

v3: Rebased

Signed-off-by: Dave Gordon 
Reviewed-by: Chris Wilson 
---
 drivers/gpu/drm/i915/intel_lrc.c | 41 +++-
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index c551c97..148d291 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -227,9 +227,8 @@ enum {
 
 static int intel_lr_context_pin(struct intel_context *ctx,
struct intel_engine_cs *engine);
-static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
-   struct drm_i915_gem_object *default_ctx_obj);
-
+static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring);
+static void lrc_teardown_hardware_status_page(struct intel_engine_cs *ring);
 
 /**
  * intel_sanitize_enable_execlists() - sanitize i915.enable_execlists
@@ -1555,8 +1554,7 @@ static int gen8_init_common_ring(struct intel_engine_cs 
*ring)
struct drm_i915_private *dev_priv = dev->dev_private;
u8 next_context_status_buffer_hw;
 
-   lrc_setup_hardware_status_page(ring,
-   
dev_priv->kernel_context->engine[ring->id].state);
+   lrc_setup_hardware_status_page(ring);
 
I915_WRITE_IMR(ring, ~(ring->irq_enable_mask | ring->irq_keep_mask));
I915_WRITE(RING_HWSTAM(ring->mmio_base), 0x);
@@ -2011,10 +2009,7 @@ void intel_logical_ring_cleanup(struct intel_engine_cs 
*ring)
i915_cmd_parser_fini_ring(ring);
i915_gem_batch_pool_fini(>batch_pool);
 
-   if (ring->status_page.obj) {
-   kunmap(kmap_to_page(ring->status_page.page_addr));
-   ring->status_page.obj = NULL;
-   }
+   lrc_teardown_hardware_status_page(ring);
 
ring->disable_lite_restore_wa = false;
ring->ctx_desc_template = 0;
@@ -2506,24 +2501,36 @@ uint32_t intel_lr_context_size(struct intel_engine_cs 
*ring)
return ret;
 }
 
-static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
-   struct drm_i915_gem_object *default_ctx_obj)
+static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring)
 {
-   struct drm_i915_private *dev_priv = ring->dev->dev_private;
+   struct drm_i915_private *dev_priv = to_i915(ring->dev);
+   struct intel_context *dctx = dev_priv->kernel_context;
+   struct drm_i915_gem_object *dctx_obj = dctx->engine[ring->id].state;
+   u64 dctx_addr = i915_gem_obj_ggtt_offset(dctx_obj);
struct page *page;
 
-   /* The HWSP is part of the default context object in LRC mode. */
-   ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(default_ctx_obj)
-   + LRC_PPHWSP_PN * PAGE_SIZE;
-   page = i915_gem_object_get_page(default_ctx_obj, LRC_PPHWSP_PN);
+   /*
+* The HWSP is part of the default context object in LRC mode.
+* Note that it doesn't contribute to the refcount!
+*/
+   page = i915_gem_object_get_page(dctx_obj, LRC_PPHWSP_PN);
ring->status_page.page_addr = kmap(page);
-   ring->status_page.obj = default_ctx_obj;
+   ring->status_page.gfx_addr = dctx_addr + LRC_PPHWSP_PN * PAGE_SIZE;
+   ring->status_page.obj = dctx_obj;
 
I915_WRITE(RING_HWS_PGA(ring->mmio_base),
(u32)ring->status_page.gfx_addr);
POSTING_READ(RING_HWS_PGA(ring->mmio_base));
 }
 
+static void lrc_teardown_hardware_status_page(struct intel_engine_cs *ring)
+{
+   if (ring->status_page.obj) {
+   kunmap(kmap_to_page(ring->status_page.page_addr));
+   ring->status_page.obj = NULL;
+   }
+}
+
 /**
  * intel_lr_context_deferred_alloc() - create the LRC specific bits of a 
context
  * @ctx: LR context to create.
-- 
1.9.1

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

[Intel-gfx] [PATCH v8 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6

[Sagar Arun Kamble]

RC6 setup is shared between BIOS and Driver. BIOS sets up subset of RC6
setup registers. If those are not setup Driver should not enable RC6.
For implementing this, driver can check RC_CTRL0 and RC_CTRL1 values
to know if BIOS has enabled HW/SW RC6.
This will also enable user to control RC6 using BIOS settings alone.
RC6 related instability can be avoided by disabling via BIOS settings
till driver fixes it.

v2: Had placed logic in gen8 function by mistake. Fixed it.
Ensuring RPM is not enabled in case BIOS disabled RC6.

v3: Need to disable RPM if RC6 is disabled due to BIOS settings. (Daniel)
Runtime PM enabling happens before gen9_enable_rc6.
Moved the updation of enable_rc6 parameter in intel_uncore_sanitize.

v4: Added elaborate check for BIOS RC6 setup. Prepared check_pctx for bxt.
(Imre)

v5: Caching reserved stolen base and size in the driver private data.
Reorganized RC6 setup check. Moved from gen9_enable_rc6 to
intel_uncore_sanitize. (Imre)

v6: Rebasing on the patch submitted by Imre that moves gem_init_stolen
earlier in the load.

v7: Removed PWRCTX_MAXCNT_VCSUNIT1 check as it applies to SKL. (Imre)

v8: Fixed formatting and checkpatch issues. Fixed functional issue where
RC6 ctx size check was missing. (Imre)

Cc: Imre Deak 
Signed-off-by: Sagar Arun Kamble 
---
 drivers/gpu/drm/i915/i915_gem_gtt.h|  2 ++
 drivers/gpu/drm/i915/i915_gem_stolen.c |  3 ++
 drivers/gpu/drm/i915/i915_reg.h| 11 +++
 drivers/gpu/drm/i915/intel_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_pm.c| 53 --
 drivers/gpu/drm/i915/intel_uncore.c|  2 ++
 6 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index f520c90..66a6da2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -342,6 +342,8 @@ struct i915_gtt {
 
size_t stolen_size; /* Total size of stolen memory */
size_t stolen_usable_size;  /* Total size minus BIOS reserved */
+   size_t stolen_reserved_base;
+   size_t stolen_reserved_size;
u64 mappable_end;   /* End offset that we can CPU map */
struct io_mapping *mappable;/* Mapping to our CPU mappable region */
phys_addr_t mappable_base;  /* PA of our GMADR */
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index c384dc9..ba1a00d 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -458,6 +458,9 @@ int i915_gem_init_stolen(struct drm_device *dev)
return 0;
}
 
+   dev_priv->gtt.stolen_reserved_base = reserved_base;
+   dev_priv->gtt.stolen_reserved_size = reserved_size;
+
/* It is possible for the reserved area to end before the end of stolen
 * memory, so just consider the start. */
reserved_total = stolen_top - reserved_base;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c0bd691..71b1fe9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6784,6 +6784,16 @@ enum skl_disp_power_wells {
 
 #define  VLV_PMWGICZ   _MMIO(0x1300a4)
 
+#define  RC6_LOCATION  _MMIO(0xD40)
+#define   RC6_CTX_IN_DRAM  (1 << 0)
+#define  RC6_CTX_BASE  _MMIO(0xD48)
+#defineRC6_CTX_BASE_MASK   0xFFF0
+#define  PWRCTX_MAXCNT_RCSUNIT _MMIO(0x2054)
+#define  PWRCTX_MAXCNT_VCSUNIT0_MMIO(0x12054)
+#define  PWRCTX_MAXCNT_BCSUNIT _MMIO(0x22054)
+#define  PWRCTX_MAXCNT_VECSUNIT_MMIO(0x1A054)
+#define  PWRCTX_MAXCNT_VCSUNIT1_MMIO(0x1C054)
+#defineIDLE_TIME_MASK  0xF
 #define  FORCEWAKE _MMIO(0xA18C)
 #define  FORCEWAKE_VLV _MMIO(0x1300b0)
 #define  FORCEWAKE_ACK_VLV _MMIO(0x1300b4)
@@ -6922,6 +6932,7 @@ enum skl_disp_power_wells {
 #define GEN6_RPDEUC_MMIO(0xA084)
 #define GEN6_RPDEUCSW  _MMIO(0xA088)
 #define GEN6_RC_STATE  _MMIO(0xA094)
+#define   RC6_STATE(1 << 18)
 #define GEN6_RC1_WAKE_RATE_LIMIT   _MMIO(0xA098)
 #define GEN6_RC6_WAKE_RATE_LIMIT   _MMIO(0xA09C)
 #define GEN6_RC6pp_WAKE_RATE_LIMIT _MMIO(0xA0A0)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 93ba14a..1251a7a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1566,6 +1566,7 @@ void skl_wm_get_hw_state(struct drm_device *dev);
 void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
 

[Intel-gfx] [PATCH v1 1/1] drm/i915: Hold RPM reference while setting freq limits through sysfs

[Sagar Arun Kamble]

Signed-off-by: Sagar Arun Kamble 
---
 drivers/gpu/drm/i915/i915_sysfs.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c 
b/drivers/gpu/drm/i915/i915_sysfs.c
index c6188dd..bb2fd78 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -370,6 +370,8 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
 
flush_delayed_work(_priv->rps.delayed_resume_work);
 
+   intel_runtime_pm_get(dev_priv);
+
mutex_lock(_priv->rps.hw_lock);
 
val = intel_freq_opcode(dev_priv, val);
@@ -398,6 +400,8 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
 
mutex_unlock(_priv->rps.hw_lock);
 
+   intel_runtime_pm_put(dev_priv);
+
return count;
 }
 
@@ -433,6 +437,8 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 
flush_delayed_work(_priv->rps.delayed_resume_work);
 
+   intel_runtime_pm_get(dev_priv);
+
mutex_lock(_priv->rps.hw_lock);
 
val = intel_freq_opcode(dev_priv, val);
@@ -457,6 +463,8 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 
mutex_unlock(_priv->rps.hw_lock);
 
+   intel_runtime_pm_put(dev_priv);
+
return count;
 
 }
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT

[Jani Nikula]

On Fri, 05 Feb 2016, "Thulasimani, Sivakumar"  
wrote:
> On 2/4/2016 5:59 PM, Jani Nikula wrote:
>> On Thu, 04 Feb 2016, Shubhangi Shrivastava  
>> wrote:
>>> This patch sets the invert bit for hpd detection for each port
>>> based on vbt configuration. since each AOB can be designed to
>>> depend on invert bit or not, it is expected if an AOB requires
>>> invert bit, the user will set respective bit in VBT.
>>>
>>> Signed-off-by: Sivakumar Thulasimani 
>>> Signed-off-by: Durgadoss R 
>>> Signed-off-by: Shubhangi Shrivastava 
>>> ---
>>>   drivers/gpu/drm/i915/i915_irq.c | 49 
>>> +
>>>   drivers/gpu/drm/i915/i915_reg.h |  9 
>>>   2 files changed, 58 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_irq.c 
>>> b/drivers/gpu/drm/i915/i915_irq.c
>>> index 25a8937..305e6dd 100644
>>> --- a/drivers/gpu/drm/i915/i915_irq.c
>>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>>> @@ -3424,6 +3424,54 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
>>> I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
>>>   }
>>>   
>>> +/*
>>> + * For BXT invert bit has to be set based on AOB design
>>> + * for HPD detection logic, update it based on VBT fields.
>>> + */
>>> +static void bxt_hpd_set_invert(struct drm_device *dev, u32 hotplug_port)
>>> +{
>>> +   struct drm_i915_private *dev_priv = dev->dev_private;
>>> +   int i, reg_val, val = 0;
>>> +
>>> +   for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>>> +
>>> +   /* Proceed only if invert bit is set */
>>> +   if (dev_priv->vbt.child_dev[i].common.hpd_invert == 0)
>>> +   continue;
>>> +
>>> +   /*
>>> +* Convert dvo_port to PORT_X and set appropriate bit
>>> +* only if hotplug is enabled on that port
>>> +*/
>>> +   switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
>>> +   case DVO_PORT_DPA:
>>> +   case DVO_PORT_HDMIA:
>>> +   if (hotplug_port & BXT_DE_PORT_HP_DDIA)
>>> +   val |= BXT_DDIA_HPD_INVERT;
>>> +   break;
>>> +   case DVO_PORT_DPB:
>>> +   case DVO_PORT_HDMIB:
>>> +   if (hotplug_port & BXT_DE_PORT_HP_DDIB)
>>> +   val |= BXT_DDIB_HPD_INVERT;
>>> +   break;
>>> +   case DVO_PORT_DPC:
>>> +   case DVO_PORT_HDMIC:
>>> +   if (hotplug_port & BXT_DE_PORT_HP_DDIC)
>>> +   val |= BXT_DDIC_HPD_INVERT;
>>> +   break;
>>> +   default:
>>> +   DRM_ERROR("HPD invert set for invalid dvo port %d\n",
>>> +  dev_priv->vbt.child_dev[i].common.dvo_port);
>>> +   break;
>>> +   }
>>> +   }
>>> +   reg_val = I915_READ(BXT_HOTPLUG_CTL);
>>> +   DRM_DEBUG_KMS("Invert bit setting: hp_ctl:%x hp_port:%x val:%x\n",
>>> +   reg_val, hotplug_port, val);
>>> +   reg_val &= ~BXT_DDI_HPD_INVERT_MASK;
>>> +   I915_WRITE(BXT_HOTPLUG_CTL, reg_val | val);
>>> +}
>> No, we don't want this here. Separate VBT parsing from the rest of the
>> logic. See [1] for some directions where I want to take this type of
>> things.
> hmm understood, will add intel_bios_requires_invert(dev, port)
> and change the logic above to
> if (intel_bios_requires_invert(dev,port)
>  val |= port;
> hope this should be fine.

I'd make it

bool intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv, enum 
port port);

BR,
Jani.



>> BR,
>> Jani.
>>
>> [1] http://mid.gmane.org/cover.1452541881.git.jani.nik...@intel.com
>>
>>
>>
>>> +
>>>   static void spt_hpd_irq_setup(struct drm_device *dev)
>>>   {
>>> struct drm_i915_private *dev_priv = dev->dev_private;
>>> @@ -3494,6 +3542,7 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
>>> hotplug |= PORTC_HOTPLUG_ENABLE | PORTB_HOTPLUG_ENABLE |
>>> PORTA_HOTPLUG_ENABLE;
>>> I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
>>> +   bxt_hpd_set_invert(dev, enabled_irqs);
>>>   }
>>>   
>>>   static void ibx_irq_postinstall(struct drm_device *dev)
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>>> b/drivers/gpu/drm/i915/i915_reg.h
>>> index 0a98889..01bd3c5 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -5936,6 +5936,15 @@ enum skl_disp_power_wells {
>>>   #define GEN8_PCU_IIR _MMIO(0x444e8)
>>>   #define GEN8_PCU_IER _MMIO(0x444ec)
>>>   
>>> +/* BXT hotplug control */
>>> +#define BXT_HOTPLUG_CTL_MMIO(0xC4030)
>>> +#define BXT_DDIA_HPD_INVERT(1 << 27)
>>> +#define BXT_DDIC_HPD_INVERT(1 << 11)
>>> +#define BXT_DDIB_HPD_INVERT(1 << 3)
>>> +#define BXT_DDI_HPD_INVERT_MASK(BXT_DDIA_HPD_INVERT | \
>>> +

[Intel-gfx] [PATCH v2] drm/i915: Reject invalid-pad for context-destroy and -create ioctls

[Chris Wilson]

Unknown parameters, especially structure padding, are expected to invoke
rejection with -EINVAL.

v2: similar issue exists for context-create

Testcase: igt/gem_ctx_create/invalid-pad
Testcase: igt/gem_ctx_bad_destroy/invalid-pad
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89602
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93999
Signed-off-by: Chris Wilson 
Cc: Daniel Vetter 
Reviewed-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_gem_context.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 54af654d04d6..c75d3468f29a 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -972,6 +972,9 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, 
void *data,
if (!contexts_enabled(dev))
return -ENODEV;
 
+   if (args->pad != 0)
+   return -EINVAL;
+
ret = i915_mutex_lock_interruptible(dev);
if (ret)
return ret;
@@ -995,6 +998,9 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, 
void *data,
struct intel_context *ctx;
int ret;
 
+   if (args->pad != 0)
+   return -EINVAL;
+
if (args->ctx_id == DEFAULT_CONTEXT_HANDLE)
return -ENOENT;
 
-- 
2.7.0

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

[Intel-gfx] [PATCH v3] drm/i915/execlists: Move WA_TAIL_DWORDS to callee

[Dave Gordon]

From: Chris Wilson 

Currently emit-request starts writing to the ring and reserves space
for a workaround to be emitted later whilst submitting the request. It
is easier to read if the caller only allocates sufficient space for
its access (then the reader can quickly verify that the ring begin
allocates the exact space for the number of dwords emitted) and closes
the access to the ring. During submit, if we need to add the workaround,
we can reacquire ring access, in the assurance that we reserved space
for ourselves when beginning the request.

v3:
Reinstated #define for WA_TAIL_DWORDS so we could accommodate GPUs
that required different amounts of padding, generalised NOOP fill
[Rodrigo Vivi], added W/A space to reserved amount and updated
code comments [Dave Gordon],

Originally-by: Rodrigo Vivi 
Rewritten-by: Chris Wilson 
Further-tweaked-by: Dave Gordon 

Signed-off-by: Dave Gordon 
Cc: Rodrigo Vivi 
Cc: Chris Wilson 
Cc: Dave Gordon 
Cc: Ben Widawsky 
---
 drivers/gpu/drm/i915/intel_lrc.c | 80 
 1 file changed, 49 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 3a03646..8b278f1 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -225,6 +225,13 @@ enum {
 #define GEN8_CTX_ID_SHIFT 32
 #define CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT  0x17
 
+/*
+ * Reserve space for 2 NOOPs at the end of each request,
+ * to be used as a workaround for not being allowed to
+ * do lite restore with HEAD==TAIL (WaIdleLiteRestore).
+ */
+#define WA_TAIL_DWORDS 2
+
 static int intel_lr_context_pin(struct intel_context *ctx,
struct intel_engine_cs *engine);
 static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
@@ -462,10 +469,9 @@ static void execlists_context_unqueue(struct 
intel_engine_cs *ring)
 */
if (req0->elsp_submitted) {
/*
-* Apply the wa NOOPS to prevent ring:HEAD == req:TAIL
-* as we resubmit the request. See gen8_emit_request()
-* for where we prepare the padding after the end of the
-* request.
+* Consume the W/A NOOPs to prevent ring:HEAD == 
req:TAIL as
+* we resubmit the request. See 
intel_logical_ring_submit()
+* where we prepare the padding after the end of the 
request.
 */
struct intel_ringbuffer *ringbuf;
 
@@ -752,33 +758,47 @@ static int logical_ring_wait_for_space(struct 
drm_i915_gem_request *req,
 }
 
 /*
- * intel_logical_ring_advance_and_submit() - advance the tail and submit the 
workload
- * @request: Request to advance the logical ringbuffer of.
+ * intel_logical_ring_submit() - submit the workload (to GuC or execlist queue)
+ * @request: Request to submit
  *
- * The tail is updated in our logical ringbuffer struct, not in the actual 
context. What
- * really happens during submission is that the context and current tail will 
be placed
- * on a queue waiting for the ELSP to be ready to accept a new context 
submission. At that
- * point, the tail *inside* the context is updated and the ELSP written to.
+ * The tail is updated in our logical ringbuffer struct, not in the actual
+ * context. What really happens during submission is that the context and
+ * current tail will be placed on a queue waiting for the ELSP to be ready 
+ * to accept a new context submission. At that point, the tail *inside* the
+ * context is updated and the ELSP written to by the submitting agent i.e.
+ * either the driver (in execlist mode), or the GuC (in GuC-submission mode).
  */
 static int
-intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request)
+intel_logical_ring_submit(struct drm_i915_gem_request *request)
 {
struct intel_ringbuffer *ringbuf = request->ringbuf;
struct drm_i915_private *dev_priv = request->i915;
struct intel_engine_cs *engine = request->ring;
 
-   intel_logical_ring_advance(ringbuf);
request->tail = ringbuf->tail;
 
/*
-* Here we add two extra NOOPs as padding to avoid
-* lite restore of a context with HEAD==TAIL.
-*
-* Caller must reserve WA_TAIL_DWORDS for us!
+* Fill in a few NOOPs after the end of the request proper,
+* as a buffer between requests to be used as a workaround
+* for not being allowed to do lite restore with HEAD==TAIL.
+* (WaIdleLiteRestore). These words may be consumed by the
+* submission mechanism if a context is *re*submitted 

Re: [Intel-gfx] [PATCH v3] drm/i915/execlists: Move WA_TAIL_DWORDS to callee

[Rodrigo Vivi]

On Fri, Feb 5, 2016 at 11:31 AM, Dave Gordon  wrote:
> From: Chris Wilson 
>
> Currently emit-request starts writing to the ring and reserves space
> for a workaround to be emitted later whilst submitting the request. It
> is easier to read if the caller only allocates sufficient space for
> its access (then the reader can quickly verify that the ring begin
> allocates the exact space for the number of dwords emitted) and closes
> the access to the ring. During submit, if we need to add the workaround,
> we can reacquire ring access, in the assurance that we reserved space
> for ourselves when beginning the request.
>
> v3:
> Reinstated #define for WA_TAIL_DWORDS so we could accommodate GPUs
> that required different amounts of padding, generalised NOOP fill
> [Rodrigo Vivi], added W/A space to reserved amount and updated
> code comments [Dave Gordon],
>
> Originally-by: Rodrigo Vivi 
> Rewritten-by: Chris Wilson 
> Further-tweaked-by: Dave Gordon 
>
> Signed-off-by: Dave Gordon 
> Cc: Rodrigo Vivi 
> Cc: Chris Wilson 
> Cc: Dave Gordon 
> Cc: Ben Widawsky 
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 80 
> 
>  1 file changed, 49 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> b/drivers/gpu/drm/i915/intel_lrc.c
> index 3a03646..8b278f1 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -225,6 +225,13 @@ enum {
>  #define GEN8_CTX_ID_SHIFT 32
>  #define CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT  0x17
>
> +/*
> + * Reserve space for 2 NOOPs at the end of each request,
> + * to be used as a workaround for not being allowed to
> + * do lite restore with HEAD==TAIL (WaIdleLiteRestore).
> + */
> +#define WA_TAIL_DWORDS 2

This patch really organize things better, but I still don't like the
WA_TAIL_DWORDS hardcoded here instead in a place where I can switch
for different platofms.

> +
>  static int intel_lr_context_pin(struct intel_context *ctx,
> struct intel_engine_cs *engine);
>  static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
> @@ -462,10 +469,9 @@ static void execlists_context_unqueue(struct 
> intel_engine_cs *ring)
>  */
> if (req0->elsp_submitted) {
> /*
> -* Apply the wa NOOPS to prevent ring:HEAD == req:TAIL
> -* as we resubmit the request. See gen8_emit_request()
> -* for where we prepare the padding after the end of 
> the
> -* request.
> +* Consume the W/A NOOPs to prevent ring:HEAD == 
> req:TAIL as
> +* we resubmit the request. See 
> intel_logical_ring_submit()
> +* where we prepare the padding after the end of the 
> request.
>  */
> struct intel_ringbuffer *ringbuf;
>
> @@ -752,33 +758,47 @@ static int logical_ring_wait_for_space(struct 
> drm_i915_gem_request *req,
>  }
>
>  /*
> - * intel_logical_ring_advance_and_submit() - advance the tail and submit the 
> workload
> - * @request: Request to advance the logical ringbuffer of.
> + * intel_logical_ring_submit() - submit the workload (to GuC or execlist 
> queue)
> + * @request: Request to submit
>   *
> - * The tail is updated in our logical ringbuffer struct, not in the actual 
> context. What
> - * really happens during submission is that the context and current tail 
> will be placed
> - * on a queue waiting for the ELSP to be ready to accept a new context 
> submission. At that
> - * point, the tail *inside* the context is updated and the ELSP written to.
> + * The tail is updated in our logical ringbuffer struct, not in the actual
> + * context. What really happens during submission is that the context and
> + * current tail will be placed on a queue waiting for the ELSP to be ready
> + * to accept a new context submission. At that point, the tail *inside* the
> + * context is updated and the ELSP written to by the submitting agent i.e.
> + * either the driver (in execlist mode), or the GuC (in GuC-submission mode).
>   */
>  static int
> -intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request)
> +intel_logical_ring_submit(struct drm_i915_gem_request *request)
>  {
> struct intel_ringbuffer *ringbuf = request->ringbuf;
> struct drm_i915_private *dev_priv = request->i915;
> struct intel_engine_cs *engine = request->ring;
>
> -   intel_logical_ring_advance(ringbuf);
> request->tail = ringbuf->tail;
>
> /*
> -* Here we add two extra NOOPs as padding to avoid
> -  

[Intel-gfx] [PATCH v2 i-g-t] igt/list-workarounds: Extend the script to Mesa

[Sameer Kibey]

Updated the list-workarounds script so that it
can parse Mesa directory if provided. Moved the
common code to a separate function to allow
reuse for both kernel and mesa.

The new command line is:
Usage: list-workarounds [options] path-to-kernel
   -k path-to-kernel -m path-to-mesa

The legacy usage is retained to avoid breaking
backwards compatibility. New parameters -k and
-m are added for the new behavior.

Either kernel or mesa or both paths can be specified.
If path-to-mesa is invalid, error is reported.

Signed-off-by: Sameer Kibey 
---
 scripts/list-workarounds | 74 ++--
 1 file changed, 53 insertions(+), 21 deletions(-)

diff --git a/scripts/list-workarounds b/scripts/list-workarounds
index d11b6a9..8b41ae5 100755
--- a/scripts/list-workarounds
+++ b/scripts/list-workarounds
@@ -18,7 +18,7 @@ def find_nth(haystack, needle, n):
return start
 
 valid_platforms = ('ctg', 'elk', 'ilk', 'snb', 'ivb', 'vlv', 'hsw', 'bdw',
-  'chv', 'skl', 'bxt')
+  'chv', 'skl', 'bxt', 'kbl')
 def parse_platforms(line, p):
l =  p.split(',')
for p in l:
@@ -65,9 +65,15 @@ def execute(cmd):
return out, err
 
 def parse_options(args):
-   usage = "Usage: list-workarounds [options] path-to-kernel"
+   usage = "Usage: list-workarounds [options] path-to-kernel -k 
path-to-kernel -m path-to-mesa"
parser = optparse.OptionParser(usage, version=1.0)
 
+   parser.add_option("-k", "--kernel-path", dest="kernel_path", 
default=None,
+ help="path to kernel")
+
+   parser.add_option("-m", "--mesa-path", dest="mesa_path", default=None,
+ help="path to mesa")
+
parser.add_option("-v", "--verbose", action="store_true",
  dest="verbose", default=False,
  help="be more verbose")
@@ -76,38 +82,64 @@ def parse_options(args):
  help="List workarounds for the specified platform")
 
(options, args) = parser.parse_args()
-
return (options, args)
 
-if __name__ == '__main__':
-   (options, args) = parse_options(sys.argv[1:])
-   verbose = options.verbose
-
-   if not len(args):
-   sys.stderr.write("error: A path to a kernel tree is required\n")
-   sys.exit(1)
-
-   kernel_path = args[0]
-   kconfig = os.path.join(kernel_path, 'Kconfig')
-   if not os.path.isfile(kconfig):
-   sys.stderr.write("error: %s does not point to a kernel tree \n"
-% kernel_path)
-   sys.exit(1)
-
-   i915_dir = os.path.join('drivers', 'gpu', 'drm', 'i915')
+def print_workarounds(project_root, driver_dir, project):
olddir = os.getcwd()
-   os.chdir(kernel_path)
+   os.chdir(project_root)
work_arounds, err = execute(['git', 'grep', '-n',
 '-e', 'W[aA][A-Z0-9][a-zA-Z0-9_]\+',
-i915_dir])
+driver_dir])
os.chdir(olddir)
if err:
print(err)
sys.exit(1)
 
parse(work_arounds)
+   print "\nList of workarounds found in %s:" % project
for wa in sorted(workarounds.keys()):
if not options.platform:
print("%s: %s" % (wa, ', '.join(workarounds[wa])))
elif options.platform in workarounds[wa]:
print(wa)
+
+
+if __name__ == '__main__':
+   (options, args) = parse_options(sys.argv)
+   verbose = options.verbose
+   kernel_path = None
+
+   if not len(args) and options.kernel_path == None and options.mesa_path 
== None:
+   sys.stderr.write("error: A path to either a kernel tree or Mesa 
is required\n")
+   sys.exit(1)
+
+   if len(args):
+   kernel_path = args[0]
+   elif options.kernel_path != None:
+   kernel_path = options.kernel_path
+
+   if kernel_path != None:
+   # --- list Kernel workarounds if path is provided ---
+   kconfig = os.path.join(kernel_path, 'Kconfig')
+   if not os.path.isfile(kconfig):
+   sys.stderr.write("error: %s does not point to a kernel 
tree \n"
+   % kernel_path)
+   sys.exit(1)
+
+   i915_dir = os.path.join('drivers', 'gpu', 'drm', 'i915')
+   print_workarounds(kernel_path, i915_dir, "kernel")
+
+   # --- list mesa workarounds if path is provided ---
+   if options.mesa_path != None:
+   # reset workarounds array
+   workarounds = {}
+
+   mesa_path = options.mesa_path
+   i965_dir = os.path.join('src', 'mesa', 'drivers', 'dri', 'i965')
+   mesa_dir = os.path.join(mesa_path, i965_dir)
+ 

[Intel-gfx] [PATCH 2/2] drm/i915/dp: reduce missing TPS3 support errors to debug logging

[Jani Nikula]

Per spec, TPS3 support is mandatory for downstream devices that support
HBR2. We've therefore logged errors on HBR2 without TPS3 since

commit 1da7d7131c35cde83f1bab8ec732b57b69bef814
Author: Jani Nikula 
Date:   Thu Sep 3 11:16:08 2015 +0300

drm/i915: ignore link rate in TPS3 selection

However, it seems there are real world devices out there that just
aren't spec compliant, and still work at HBR2 using TPS2. So reduce the
error message to debug logging.

Cc: Ander Conselvan de Oliveira 
Cc: Sivakumar Thulasimani 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92932
Fixes: 1da7d7131c35 ("drm/i915: ignore link rate in TPS3 selection")
Cc: drm-intel-fi...@lists.freedesktop.org
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_dp_link_training.c | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c 
b/drivers/gpu/drm/i915/intel_dp_link_training.c
index 83e667b92fda..0b8eefc2acc5 100644
--- a/drivers/gpu/drm/i915/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
@@ -222,19 +222,27 @@ intel_dp_link_training_clock_recovery(struct intel_dp 
*intel_dp)
 static u32 intel_dp_training_pattern(struct intel_dp *intel_dp)
 {
u32 training_pattern = DP_TRAINING_PATTERN_2;
+   bool source_tps3, sink_tps3;
 
/*
 * Intel platforms that support HBR2 also support TPS3. TPS3 support is
-* also mandatory for downstream devices that support HBR2.
+* also mandatory for downstream devices that support HBR2. However, not
+* all sinks follow the spec.
 *
 * Due to WaDisableHBR2 SKL < B0 is the only exception where TPS3 is
-* supported but still not enabled.
+* supported in source but still not enabled.
 */
-   if (intel_dp_source_supports_hbr2(intel_dp) &&
-   drm_dp_tps3_supported(intel_dp->dpcd))
+   source_tps3 = intel_dp_source_supports_hbr2(intel_dp);
+   sink_tps3 = drm_dp_tps3_supported(intel_dp->dpcd);
+
+   if (source_tps3 && sink_tps3) {
training_pattern = DP_TRAINING_PATTERN_3;
-   else if (intel_dp->link_rate == 54)
-   DRM_ERROR("5.4 Gbps link rate without HBR2/TPS3 support\n");
+   } else if (intel_dp->link_rate == 54) {
+   if (!source_tps3)
+   DRM_DEBUG_KMS("5.4 Gbps link rate without source 
HBR2/TPS3 support\n");
+   if (!sink_tps3)
+   DRM_DEBUG_KMS("5.4 Gbps link rate without sink TPS3 
support\n");
+   }
 
return training_pattern;
 }
-- 
2.1.4

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

Re: [Intel-gfx] [PATCH:xf86-intel-video] Add NULL checking for drawable in sna_dri2_flip_event

[Chris Wilson]

On Fri, Feb 05, 2016 at 04:50:22PM +0800, Lim Siew Hoon wrote:
> The last flip complete signal may happen after the
> sna_dri2_destroy_window function has been called. This
> leads to calling frame_swap_complete with a null flip
> drawable. So check for that and handle accordingly.

Treating the symptom and not the cause.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH RESEND 3/5] drm/i915: move VBT based eDP port check to intel_bios.c

[Jani Nikula]

Hide knowledge about VBT child devices in intel_bios.c.

Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/i915_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_bios.c | 33 +
 drivers/gpu/drm/i915/intel_dp.c   | 21 +
 3 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d70ef71d2538..234f33708a31 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3399,6 +3399,7 @@ int intel_bios_init(struct drm_i915_private *dev_priv);
 bool intel_bios_is_valid_vbt(const void *buf, size_t size);
 bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
 bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 
*i2c_pin);
+bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
 
 /* intel_opregion.c */
 #ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/intel_bios.c 
b/drivers/gpu/drm/i915/intel_bios.c
index f3f4f8e687cf..1429d8214885 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1519,3 +1519,36 @@ bool intel_bios_is_lvds_present(struct drm_i915_private 
*dev_priv, u8 *i2c_pin)
 
return false;
 }
+
+/**
+ * intel_bios_is_port_edp - is the device in given port eDP
+ * @dev_priv:  i915 device instance
+ * @port:  port to check
+ *
+ * Return true if the device in %port is eDP.
+ */
+bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
+{
+   union child_device_config *p_child;
+   static const short port_mapping[] = {
+   [PORT_B] = DVO_PORT_DPB,
+   [PORT_C] = DVO_PORT_DPC,
+   [PORT_D] = DVO_PORT_DPD,
+   [PORT_E] = DVO_PORT_DPE,
+   };
+   int i;
+
+   if (!dev_priv->vbt.child_dev_num)
+   return false;
+
+   for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+   p_child = dev_priv->vbt.child_dev + i;
+
+   if (p_child->common.dvo_port == port_mapping[port] &&
+   (p_child->common.device_type & DEVICE_TYPE_eDP_BITS) ==
+   (DEVICE_TYPE_eDP & DEVICE_TYPE_eDP_BITS))
+   return true;
+   }
+
+   return false;
+}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a073f04a5330..9a4cfdf0cd70 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5065,14 +5065,6 @@ put_power:
 bool intel_dp_is_edp(struct drm_device *dev, enum port port)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
-   union child_device_config *p_child;
-   int i;
-   static const short port_mapping[] = {
-   [PORT_B] = DVO_PORT_DPB,
-   [PORT_C] = DVO_PORT_DPC,
-   [PORT_D] = DVO_PORT_DPD,
-   [PORT_E] = DVO_PORT_DPE,
-   };
 
/*
 * eDP not supported on g4x. so bail out early just
@@ -5084,18 +5076,7 @@ bool intel_dp_is_edp(struct drm_device *dev, enum port 
port)
if (port == PORT_A)
return true;
 
-   if (!dev_priv->vbt.child_dev_num)
-   return false;
-
-   for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
-   p_child = dev_priv->vbt.child_dev + i;
-
-   if (p_child->common.dvo_port == port_mapping[port] &&
-   (p_child->common.device_type & DEVICE_TYPE_eDP_BITS) ==
-   (DEVICE_TYPE_eDP & DEVICE_TYPE_eDP_BITS))
-   return true;
-   }
-   return false;
+   return intel_bios_is_port_edp(dev_priv, port);
 }
 
 void
-- 
2.1.4

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

Re: [Intel-gfx] [RFC 02/29] drm/i915: Introduce host graphics memory balloon for gvt

[Zhiyuan Lv]

Hi Joonas,

Thanks much for the review! We will incorporate those review comments!

Meanwhile, is it good enough to do the host ballooning like below? The
pros is that it is very simple, especially consider that guest
ballooning logic has been there. Thanks!

Regards,
-Zhiyuan

On Thu, Feb 04, 2016 at 01:27:08PM +0200, Joonas Lahtinen wrote:
> Hi,
> 
> On to, 2016-01-28 at 18:21 +0800, Zhi Wang wrote:
> > From: Bing Niu 
> > 
> > This patch introduces host graphics memory ballon when GVT-g is enabled.
> > 
> > As under GVT-g, i915 only owned limited graphics resources, others are
> > managed by GVT-g resource allocator and kept for other vGPUs.
> > 
> > For graphics memory space partition, a typical layout looks like:
> > 
> > +---+---+--+---+
> > > * Host |   *GVT-g Resource |* Host|   *GVT-g Resource |
> > > Owned |   Allocator Managed   | Owned|   Allocator Managed   |
> > >   |   |  |   |
> > +---+---+--+---+---+
> > >   |   |   |   |  |   |   |   |
> > > i915  | vm 1  | vm 2  | vm 3  | i915 | vm 1  | vm 2  | vm 3  |
> > >   |   |   |   |  |   |   |   |
> > +---+---+---+--+---+---+---+
> > >   Aperture|Hidden|
> > +---+--+
> > >   GGTT memory space  |
> > +--+
> > 
> > Similar with fence registers partition:
> > 
> >  +-- +---+
> >  | * Host|GVT-g Resource |
> >  | Owned |   Allocator Managed   +
> >  0   |   31
> >  +---+---+---+
> >  |   |   |   |   |
> >  | i915  | vm 1  | vm 2  | vm 3  |
> >  |   |   |   |   |
> >  +---+---+---+---+
> > 
> > i915 host will read the amount of allocated resources via GVT-g kernel 
> > parameters.
> > 
> > Signed-off-by: Bing Niu 
> > Signed-off-by: Zhi Wang 
> > ---
> >  drivers/gpu/drm/i915/gvt/params.h   |  3 +++
> >  drivers/gpu/drm/i915/i915_gem.c |  3 +++
> >  drivers/gpu/drm/i915/i915_gem_gtt.c |  4 ++--
> >  drivers/gpu/drm/i915/i915_vgpu.c| 16 
> >  drivers/gpu/drm/i915/i915_vgpu.h|  1 +
> >  5 files changed, 21 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/params.h 
> > b/drivers/gpu/drm/i915/gvt/params.h
> > index d2955b9..0656a98 100644
> > --- a/drivers/gpu/drm/i915/gvt/params.h
> > +++ b/drivers/gpu/drm/i915/gvt/params.h
> > @@ -27,6 +27,9 @@
> >  struct gvt_kernel_params {
> >     bool enable;
> >     int debug;
> > +   int dom0_low_gm_sz;
> > +   int dom0_high_gm_sz;
> > +   int dom0_fence_sz;
> >  };
> >  
> >  extern struct gvt_kernel_params gvt;
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c 
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index 799a53a..e916e43 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -5080,6 +5080,9 @@ i915_gem_load(struct drm_device *dev)
> >     else
> >     dev_priv->num_fence_regs = 8;
> >  
> > +   if(intel_gvt_host_active(dev))
> 
> Space between "if" and "("

Thanks! Will correct it.

> 
> > +   dev_priv->num_fence_regs = gvt.dom0_fence_sz;
> > +
> >     if (intel_vgpu_active(dev))
> >     dev_priv->num_fence_regs =
> >     I915_READ(vgtif_reg(avail_rs.fence_num));
> 
> I'd like to see the above as "else if" construct just like you have
> below with the intel_vgt_balloon(). Could even do
> 
>   if (gvt_host)  {
>   ...
>   } else if (vgpu_active) {
>   ...
>   } else {
>   per machine detection
>   }
> 
> Right?

That looks better!

> 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> > b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index 7377b67..0540de2 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -2713,7 +2713,7 @@ static int i915_gem_setup_global_gtt(struct 
> > drm_device *dev,
> >     i915_address_space_init(ggtt_vm, dev_priv);
> >     ggtt_vm->total += PAGE_SIZE;
> >  
> > -   if (intel_vgpu_active(dev)) {
> > +   if (intel_vgpu_active(dev) || intel_gvt_host_active(dev)) {
> >     ret = intel_vgt_balloon(dev);
> >     if (ret)
> >     return ret;
> > @@ -2810,7 +2810,7 @@ void i915_global_gtt_cleanup(struct drm_device *dev)
> >     }
> >  
> >     if (drm_mm_initialized(>mm)) {
> > -   if (intel_vgpu_active(dev))
> > +   if (intel_vgpu_active(dev) || intel_gvt_host_active(dev))
> >     intel_vgt_deballoon();
> >  
> >     drm_mm_takedown(>mm);
> > diff 

[Intel-gfx] [PATCH RESEND 4/5] drm/i915: move VBT based DSI presence check to intel_bios.c

[Jani Nikula]

Hide knowledge about VBT child devices in intel_bios.c.

Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/i915_drv.h   |  2 +-
 drivers/gpu/drm/i915/intel_bios.c | 33 -
 drivers/gpu/drm/i915/intel_dsi.c  | 23 ++-
 3 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 234f33708a31..cd2595c25f8d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1516,7 +1516,6 @@ struct intel_vbt_data {
 
/* MIPI DSI */
struct {
-   u16 port;
u16 panel_id;
struct mipi_config *config;
struct mipi_pps_data *pps;
@@ -3400,6 +3399,7 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t 
size);
 bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
 bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 
*i2c_pin);
 bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
+bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port 
*port);
 
 /* intel_opregion.c */
 #ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/intel_bios.c 
b/drivers/gpu/drm/i915/intel_bios.c
index 1429d8214885..b0cf33234c33 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1237,7 +1237,6 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
&_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT) {
DRM_DEBUG_KMS("Found MIPI as LFP\n");
dev_priv->vbt.has_mipi = 1;
-   dev_priv->vbt.dsi.port = p_child->common.dvo_port;
}
 
child_dev_ptr = dev_priv->vbt.child_dev + count;
@@ -1552,3 +1551,35 @@ bool intel_bios_is_port_edp(struct drm_i915_private 
*dev_priv, enum port port)
 
return false;
 }
+
+/**
+ * intel_bios_is_dsi_present - is DSI present in VBT
+ * @dev_priv:  i915 device instance
+ * @port:  port for DSI if present
+ *
+ * Return true if DSI is present, and return the port in %port.
+ */
+bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv,
+  enum port *port)
+{
+   union child_device_config *p_child;
+   int i;
+
+   for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+   p_child = dev_priv->vbt.child_dev + i;
+
+   if (!(p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT))
+   continue;
+
+   switch (p_child->common.dvo_port) {
+   case DVO_PORT_MIPIA:
+   case DVO_PORT_MIPIB:
+   case DVO_PORT_MIPIC:
+   case DVO_PORT_MIPID:
+   *port = p_child->common.dvo_port - DVO_PORT_MIPIA;
+   return true;
+   }
+   }
+
+   return false;
+}
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 378f879f4015..5b6ec567e3ce 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1112,9 +1112,15 @@ void intel_dsi_init(struct drm_device *dev)
DRM_DEBUG_KMS("\n");
 
/* There is no detection method for MIPI so rely on VBT */
-   if (!dev_priv->vbt.has_mipi)
+   if (!intel_bios_is_dsi_present(dev_priv, ))
return;
 
+   if (port != PORT_A && port != PORT_C) {
+   DRM_DEBUG_KMS("VBT has unsupported DSI port %c\n",
+ port_name(port));
+   return;
+   }
+
if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
dev_priv->mipi_mmio_base = VLV_MIPI_BASE;
} else {
@@ -1153,16 +1159,15 @@ void intel_dsi_init(struct drm_device *dev)
intel_connector->unregister = intel_connector_unregister;
 
/* Pipe A maps to MIPI DSI port A, pipe B maps to MIPI DSI port C */
-   if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) {
-   intel_encoder->crtc_mask = (1 << PIPE_A);
-   intel_dsi->ports = (1 << PORT_A);
-   } else if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIC) {
-   intel_encoder->crtc_mask = (1 << PIPE_B);
-   intel_dsi->ports = (1 << PORT_C);
-   }
+   if (port == PORT_A)
+   intel_encoder->crtc_mask = 1 << PIPE_A;
+   else
+   intel_encoder->crtc_mask = 1 << PIPE_B;
 
if (dev_priv->vbt.dsi.config->dual_link)
-   intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
+   intel_dsi->ports = (1 << PORT_A) | (1 << PORT_C);
+   else
+   intel_dsi->ports = 1 << port;
 
/* Create a DSI host (and a device) for each port. */
for_each_dsi_port(port, intel_dsi->ports) {
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org

[Intel-gfx] [PATCH RESEND 5/5] drm/i915/panel: setup pwm backlight based on connector type

[Jani Nikula]

Use the connector type instead of VBT directly to decide which backlight
mechanism to use on VLV/CHV. (Indirectly, this is the same thing, but
hides the VBT use.)

Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_panel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_panel.c 
b/drivers/gpu/drm/i915/intel_panel.c
index 21ee6477bf98..f7fbb7c223b1 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1745,7 +1745,7 @@ intel_panel_init_backlight_funcs(struct intel_panel 
*panel)
panel->backlight.get = pch_get_backlight;
panel->backlight.hz_to_pwm = pch_hz_to_pwm;
} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
-   if (dev_priv->vbt.has_mipi) {
+   if (connector->base.connector_type == DRM_MODE_CONNECTOR_DSI) {
panel->backlight.setup = pwm_setup_backlight;
panel->backlight.enable = pwm_enable_backlight;
panel->backlight.disable = pwm_disable_backlight;
-- 
2.1.4

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

[Intel-gfx] [PATCH RESEND 1/5] drm/i915: move VBT based TV presence check to intel_bios.c

[Jani Nikula]

Hide knowledge about VBT child devices in intel_bios.c.

Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/i915_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_bios.c | 38 ++
 drivers/gpu/drm/i915/intel_tv.c   | 43 +--
 3 files changed, 40 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 77227a39f3d5..715f200cfbf5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3397,6 +3397,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
 /* intel_bios.c */
 int intel_bios_init(struct drm_i915_private *dev_priv);
 bool intel_bios_is_valid_vbt(const void *buf, size_t size);
+bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
 
 /* intel_opregion.c */
 #ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/intel_bios.c 
b/drivers/gpu/drm/i915/intel_bios.c
index bf62a19c8f69..2800ae50465a 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1431,3 +1431,41 @@ intel_bios_init(struct drm_i915_private *dev_priv)
 
return 0;
 }
+
+/**
+ * intel_bios_is_tv_present - is integrated TV present in VBT
+ * @dev_priv:  i915 device instance
+ *
+ * Return true if TV is present. If no child devices were parsed from VBT,
+ * assume TV is present.
+ */
+bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
+{
+   union child_device_config *p_child;
+   int i;
+
+   if (!dev_priv->vbt.child_dev_num)
+   return true;
+
+   for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+   p_child = dev_priv->vbt.child_dev + i;
+   /*
+* If the device type is not TV, continue.
+*/
+   switch (p_child->old.device_type) {
+   case DEVICE_TYPE_INT_TV:
+   case DEVICE_TYPE_TV:
+   case DEVICE_TYPE_TV_SVIDEO_COMPOSITE:
+   break;
+   default:
+   continue;
+   }
+   /* Only when the addin_offset is non-zero, it is regarded
+* as present.
+*/
+   if (p_child->old.addin_offset)
+   return true;
+   }
+
+   return false;
+}
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 5034b0055169..8417fcad02d2 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1530,47 +1530,6 @@ static const struct drm_encoder_funcs intel_tv_enc_funcs 
= {
.destroy = intel_encoder_destroy,
 };
 
-/*
- * Enumerate the child dev array parsed from VBT to check whether
- * the integrated TV is present.
- * If it is present, return 1.
- * If it is not present, return false.
- * If no child dev is parsed from VBT, it assumes that the TV is present.
- */
-static int tv_is_present_in_vbt(struct drm_device *dev)
-{
-   struct drm_i915_private *dev_priv = dev->dev_private;
-   union child_device_config *p_child;
-   int i, ret;
-
-   if (!dev_priv->vbt.child_dev_num)
-   return 1;
-
-   ret = 0;
-   for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
-   p_child = dev_priv->vbt.child_dev + i;
-   /*
-* If the device type is not TV, continue.
-*/
-   switch (p_child->old.device_type) {
-   case DEVICE_TYPE_INT_TV:
-   case DEVICE_TYPE_TV:
-   case DEVICE_TYPE_TV_SVIDEO_COMPOSITE:
-   break;
-   default:
-   continue;
-   }
-   /* Only when the addin_offset is non-zero, it is regarded
-* as present.
-*/
-   if (p_child->old.addin_offset) {
-   ret = 1;
-   break;
-   }
-   }
-   return ret;
-}
-
 void
 intel_tv_init(struct drm_device *dev)
 {
@@ -1586,7 +1545,7 @@ intel_tv_init(struct drm_device *dev)
if ((I915_READ(TV_CTL) & TV_FUSE_STATE_MASK) == TV_FUSE_STATE_DISABLED)
return;
 
-   if (!tv_is_present_in_vbt(dev)) {
+   if (!intel_bios_is_tv_present(dev_priv)) {
DRM_DEBUG_KMS("Integrated TV is not present.\n");
return;
}
-- 
2.1.4

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

[Intel-gfx] [PATCH RESEND 2/5] drm/i915: move VBT based LVDS presence check to intel_bios.c

[Jani Nikula]

Hide knowledge about VBT child devices in intel_bios.c.

Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/i915_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_bios.c | 50 
 drivers/gpu/drm/i915/intel_lvds.c | 53 +--
 3 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 715f200cfbf5..d70ef71d2538 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3398,6 +3398,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
 int intel_bios_init(struct drm_i915_private *dev_priv);
 bool intel_bios_is_valid_vbt(const void *buf, size_t size);
 bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
+bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 
*i2c_pin);
 
 /* intel_opregion.c */
 #ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/intel_bios.c 
b/drivers/gpu/drm/i915/intel_bios.c
index 2800ae50465a..f3f4f8e687cf 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1469,3 +1469,53 @@ bool intel_bios_is_tv_present(struct drm_i915_private 
*dev_priv)
 
return false;
 }
+
+/**
+ * intel_bios_is_lvds_present - is LVDS present in VBT
+ * @dev_priv:  i915 device instance
+ * @i2c_pin:   i2c pin for LVDS if present
+ *
+ * Return true if LVDS is present. If no child devices were parsed from VBT,
+ * assume LVDS is present.
+ */
+bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
+{
+   int i;
+
+   if (!dev_priv->vbt.child_dev_num)
+   return true;
+
+   for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+   union child_device_config *uchild = dev_priv->vbt.child_dev + i;
+   struct old_child_dev_config *child = >old;
+
+   /* If the device type is not LFP, continue.
+* We have to check both the new identifiers as well as the
+* old for compatibility with some BIOSes.
+*/
+   if (child->device_type != DEVICE_TYPE_INT_LFP &&
+   child->device_type != DEVICE_TYPE_LFP)
+   continue;
+
+   if (intel_gmbus_is_valid_pin(dev_priv, child->i2c_pin))
+   *i2c_pin = child->i2c_pin;
+
+   /* However, we cannot trust the BIOS writers to populate
+* the VBT correctly.  Since LVDS requires additional
+* information from AIM blocks, a non-zero addin offset is
+* a good indicator that the LVDS is actually present.
+*/
+   if (child->addin_offset)
+   return true;
+
+   /* But even then some BIOS writers perform some black magic
+* and instantiate the device without reference to any
+* additional data.  Trust that if the VBT was written into
+* the OpRegion then they have validated the LVDS's existence.
+*/
+   if (dev_priv->opregion.vbt)
+   return true;
+   }
+
+   return false;
+}
diff --git a/drivers/gpu/drm/i915/intel_lvds.c 
b/drivers/gpu/drm/i915/intel_lvds.c
index 0da0240caf81..80f8684e5137 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -772,57 +772,6 @@ static const struct dmi_system_id intel_no_lvds[] = {
{ } /* terminating entry */
 };
 
-/*
- * Enumerate the child dev array parsed from VBT to check whether
- * the LVDS is present.
- * If it is present, return 1.
- * If it is not present, return false.
- * If no child dev is parsed from VBT, it assumes that the LVDS is present.
- */
-static bool lvds_is_present_in_vbt(struct drm_device *dev,
-  u8 *i2c_pin)
-{
-   struct drm_i915_private *dev_priv = dev->dev_private;
-   int i;
-
-   if (!dev_priv->vbt.child_dev_num)
-   return true;
-
-   for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
-   union child_device_config *uchild = dev_priv->vbt.child_dev + i;
-   struct old_child_dev_config *child = >old;
-
-   /* If the device type is not LFP, continue.
-* We have to check both the new identifiers as well as the
-* old for compatibility with some BIOSes.
-*/
-   if (child->device_type != DEVICE_TYPE_INT_LFP &&
-   child->device_type != DEVICE_TYPE_LFP)
-   continue;
-
-   if (intel_gmbus_is_valid_pin(dev_priv, child->i2c_pin))
-   *i2c_pin = child->i2c_pin;
-
-   /* However, we cannot trust the BIOS writers to populate
-* the VBT correctly.  Since LVDS requires additional
-* information from AIM blocks, a non-zero addin offset is
-

Re: [Intel-gfx] [PATCH] drm/i915: Protect fbdev across slow or failed initialisation

[Chris Wilson]

On Fri, Feb 05, 2016 at 01:27:10AM +0100, Lukas Wunner wrote:
> Hi,
> 
> On Thu, Feb 04, 2016 at 09:21:17AM +, Li, Weinan Z wrote:
> > We still need this patch. Seems 54632abe8ca3 ("drm/i915: Fix oops caused by 
> > fbdev initialization
> > failure") as well as 366e39b4d2c5 ("drm/i915: Tear down fbdev if
> > initialization fails") this 2 patches can???t cover this case. It???s 
> > access ifbdev->fb before the initialization
> > finished, but not initialization failed. If don???t have any other patches 
> > or code update relative, it may still have in 4.5.
> 
> Okay, I see.
> 
> > 
> > add info NULL check should be better, it is also initialized in the async 
> > queue
> > >   info = ifbdev->helper.fbdev;
> > > + if (info == NULL)
> > > +return false;
> > >   if (!info->screen_base)
> 
> So if there's indeed a race between fbdev initialization and the call to
> intel_fbdev_restore_mode() (on lastclose), there's more that can go awry:
> - intel_fbdev_restore_mode() dereferences ifbdev->fb->obj
> - it calls __drm_atomic_helper_set_config() which WARNs if ifbdev->fb is NULL
> - it calls drm_fb_helper_set_par() which dereferences ifbdev->helper.fbdev
> 
> Instead of checking all that it's probably simpler to call
> async_synchronize_full() at the beginning of intel_fbdev_restore_mode(),
> as Li Weinan suggested. I'll submit the corresponding one-liner patch
> tomorrow if noone else does.
> 
> Chris' patch also modified intel_fbdev_set_suspend() as well as
> intel_fbdev_output_poll_changed(), not sure if these are racy as well.
> At least the stack traces posted by Li Weinan and Gustav Fägerlind
> only indicate that lastclose is racy.

We called set-suspend internally, but we don't do any checks before
hand. I was concerned we may be able to get into a situation where
.fb_probe fails and leaves a dangling dev_priv->ifbdev for which we
would then trip over the NULL info->screen_base. So I was looking for a
suitable guard.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Skip DDI PLL selection for DSI

[Patchwork]

== Summary ==

Series 3122v1 drm/i915: Skip DDI PLL selection for DSI
http://patchwork.freedesktop.org/api/1.0/series/3122/revisions/1/mbox/

Test gem_sync:
Subgroup basic-blt:
pass   -> INCOMPLETE (skl-i5k-2)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
dmesg-warn -> PASS   (ilk-hp8440p) UNSTABLE

bdw-nuci7total:161  pass:152  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultratotal:164  pass:152  dwarn:0   dfail:0   fail:0   skip:12 
byt-nuc  total:164  pass:141  dwarn:0   dfail:0   fail:0   skip:23 
hsw-brixbox  total:164  pass:151  dwarn:0   dfail:0   fail:0   skip:13 
hsw-gt2  total:164  pass:154  dwarn:0   dfail:0   fail:0   skip:10 
ilk-hp8440p  total:164  pass:116  dwarn:0   dfail:0   fail:0   skip:48 
ivb-t430stotal:164  pass:150  dwarn:0   dfail:0   fail:0   skip:14 
skl-i5k-2total:57   pass:48   dwarn:0   dfail:0   fail:0   skip:8  
skl-i7k-2total:164  pass:149  dwarn:1   dfail:0   fail:0   skip:14 
snb-dellxps  total:164  pass:142  dwarn:0   dfail:0   fail:0   skip:22 

Results at /archive/results/CI_IGT_test/Patchwork_1369/

57e229193395068adcb34c5266d54194e652869f drm-intel-nightly: 
2016y-02m-04d-18h-38m-55s UTC integration manifest
aa42a6ab884b76ece511db816e90b80b6bd3a7ef drm/i915: Skip DDI PLL selection for 
DSI

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

[Intel-gfx] [PATCH 2/2] drm/i915: Add Backlight Control using DPCD for eDP connectors (v6)

[Yetunde Adebisi]

This patch adds support for eDP backlight control using DPCD registers to
backlight hooks in intel_panel.

It checks for backlight control over AUX channel capability and sets up
function pointers to get and set the backlight brightness level if
supported.

v2: Moved backlight functions from intel_dp.c into a new file
intel_dp_aux_backlight.c. Also moved reading of eDP display control
registers to intel_dp_get_dpcd

v3: Correct some formatting mistakes

v4: Updated to use AUX backlight control if PWM control is not possible
(Jani)
v5: Moved call to initialize backlight registers to dp_aux_setup_backlight
v6: Check DP_EDP_BACKLIGHT_PIN_ENABLE_CAP is disabled before setting up AUX
backlight control. To fix BLM_PWM_ENABLE igt test warnings on bdw_ultra

This patch depends on http://patchwork.freedesktop.org/patch/64253/

Cc: Bob Paauwe 
Cc: Jani Nikula 
Acked-by: Jesse Barnes 
Signed-off-by: Yetunde Adebisi 
---
 drivers/gpu/drm/i915/Makefile |   1 +
 drivers/gpu/drm/i915/intel_dp.c   |  17 ++-
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 170 ++
 drivers/gpu/drm/i915/intel_drv.h  |   6 +
 drivers/gpu/drm/i915/intel_panel.c|   4 +
 5 files changed, 192 insertions(+), 6 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_dp_aux_backlight.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 0851de07..41250cc 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -77,6 +77,7 @@ i915-y += dvo_ch7017.o \
  dvo_tfp410.o \
  intel_crt.o \
  intel_ddi.o \
+ intel_dp_aux_backlight.o \
  intel_dp_link_training.o \
  intel_dp_mst.o \
  intel_dp.o \
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a073f04..9f8672e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3183,7 +3183,7 @@ static void chv_dp_post_pll_disable(struct intel_encoder 
*encoder)
  * Sinks are *supposed* to come up within 1ms from an off state, but we're also
  * supposed to retry 3 times per the spec.
  */
-static ssize_t
+ssize_t
 intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset,
void *buffer, size_t size)
 {
@@ -3850,7 +3850,6 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = dig_port->base.base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
-   uint8_t rev;
 
if (intel_dp_dpcd_read_wake(_dp->aux, 0x000, intel_dp->dpcd,
sizeof(intel_dp->dpcd)) < 0)
@@ -3886,6 +3885,15 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
DRM_DEBUG_KMS("PSR2 %s on sink",
dev_priv->psr.psr2_support ? "supported" : "not 
supported");
}
+
+   /* Read the eDP Display control capabilities registers */
+   memset(intel_dp->edp_dpcd, 0, sizeof(intel_dp->edp_dpcd));
+   if ((intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] & 
DP_DPCD_DISPLAY_CONTROL_CAPABLE) &&
+   (intel_dp_dpcd_read_wake(_dp->aux, 
DP_EDP_DPCD_REV,
+   intel_dp->edp_dpcd, 
sizeof(intel_dp->edp_dpcd)) ==
+   
sizeof(intel_dp->edp_dpcd)))
+   DRM_DEBUG_KMS("EDP DPCD : %*ph\n", (int) 
sizeof(intel_dp->edp_dpcd),
+   intel_dp->edp_dpcd);
}
 
DRM_DEBUG_KMS("Display Port TPS3 support: source %s, sink %s\n",
@@ -3893,10 +3901,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
  yesno(drm_dp_tps3_supported(intel_dp->dpcd)));
 
/* Intermediate frequency support */
-   if (is_edp(intel_dp) &&
-   (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] & 
DP_DPCD_DISPLAY_CONTROL_CAPABLE) &&
-   (intel_dp_dpcd_read_wake(_dp->aux, DP_EDP_DPCD_REV, , 1) 
== 1) &&
-   (rev >= 0x03)) { /* eDp v1.4 or higher */
+   if (is_edp(intel_dp) && (intel_dp->edp_dpcd[0] >= 0x03)) { /* eDp v1.4 
or higher */
__le16 sink_rates[DP_MAX_SUPPORTED_RATES];
int i;
 
diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c 
b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
new file mode 100644
index 000..a5361d6
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -0,0 +1,170 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights 

Re: [Intel-gfx] [PATCH 2/2] drm/i915/dp: reduce missing TPS3 support errors to debug logging

[Thulasimani, Sivakumar]

Reviewed-by: Sivakumar Thulasimani 

On 2/5/2016 3:46 PM, Jani Nikula wrote:

Per spec, TPS3 support is mandatory for downstream devices that support
HBR2. We've therefore logged errors on HBR2 without TPS3 since

commit 1da7d7131c35cde83f1bab8ec732b57b69bef814
Author: Jani Nikula 
Date:   Thu Sep 3 11:16:08 2015 +0300

 drm/i915: ignore link rate in TPS3 selection

However, it seems there are real world devices out there that just
aren't spec compliant, and still work at HBR2 using TPS2. So reduce the
error message to debug logging.

Cc: Ander Conselvan de Oliveira 
Cc: Sivakumar Thulasimani 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92932
Fixes: 1da7d7131c35 ("drm/i915: ignore link rate in TPS3 selection")
Cc: drm-intel-fi...@lists.freedesktop.org
Signed-off-by: Jani Nikula 
---
  drivers/gpu/drm/i915/intel_dp_link_training.c | 20 ++--
  1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c 
b/drivers/gpu/drm/i915/intel_dp_link_training.c
index 83e667b92fda..0b8eefc2acc5 100644
--- a/drivers/gpu/drm/i915/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
@@ -222,19 +222,27 @@ intel_dp_link_training_clock_recovery(struct intel_dp 
*intel_dp)
  static u32 intel_dp_training_pattern(struct intel_dp *intel_dp)
  {
u32 training_pattern = DP_TRAINING_PATTERN_2;
+   bool source_tps3, sink_tps3;
  
  	/*

 * Intel platforms that support HBR2 also support TPS3. TPS3 support is
-* also mandatory for downstream devices that support HBR2.
+* also mandatory for downstream devices that support HBR2. However, not
+* all sinks follow the spec.
 *
 * Due to WaDisableHBR2 SKL < B0 is the only exception where TPS3 is
-* supported but still not enabled.
+* supported in source but still not enabled.
 */
-   if (intel_dp_source_supports_hbr2(intel_dp) &&
-   drm_dp_tps3_supported(intel_dp->dpcd))
+   source_tps3 = intel_dp_source_supports_hbr2(intel_dp);
+   sink_tps3 = drm_dp_tps3_supported(intel_dp->dpcd);
+
+   if (source_tps3 && sink_tps3) {
training_pattern = DP_TRAINING_PATTERN_3;
-   else if (intel_dp->link_rate == 54)
-   DRM_ERROR("5.4 Gbps link rate without HBR2/TPS3 support\n");
+   } else if (intel_dp->link_rate == 54) {
+   if (!source_tps3)
+   DRM_DEBUG_KMS("5.4 Gbps link rate without source HBR2/TPS3 
support\n");
+   if (!sink_tps3)
+   DRM_DEBUG_KMS("5.4 Gbps link rate without sink TPS3 
support\n");
+   }
  
  	return training_pattern;

  }


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

Re: [Intel-gfx] [PATCH 1/2] drm/i915/dp: abstract training pattern selection

[Jani Nikula]

On Fri, 05 Feb 2016, "Thulasimani, Sivakumar"  
wrote:
> Reviewed-by: Sivakumar Thulasimani 

Both pushed to drm-intel-next-queued, thanks for the review.

BR,
Jani.



>
> On 2/5/2016 3:46 PM, Jani Nikula wrote:
>> Make it cleaner to add more checks in the function. No functional
>> changes.
>>
>> Cc: Ander Conselvan de Oliveira 
>> Cc: Sivakumar Thulasimani 
>> Cc: drm-intel-fi...@lists.freedesktop.org # dependency on the next patch
>> Signed-off-by: Jani Nikula 
>> ---
>>   drivers/gpu/drm/i915/intel_dp_link_training.c | 25 
>> ++---
>>   1 file changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c 
>> b/drivers/gpu/drm/i915/intel_dp_link_training.c
>> index 7938e0bf..83e667b92fda 100644
>> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
>> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
>> @@ -215,16 +215,15 @@ intel_dp_link_training_clock_recovery(struct intel_dp 
>> *intel_dp)
>>  }
>>   }
>>   
>> -static void
>> -intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
>> +/*
>> + * Pick training pattern for channel equalization. Training Pattern 3 for 
>> HBR2
>> + * or 1.2 devices that support it, Training Pattern 2 otherwise.
>> + */
>> +static u32 intel_dp_training_pattern(struct intel_dp *intel_dp)
>>   {
>> -bool channel_eq = false;
>> -int tries, cr_tries;
>> -uint32_t training_pattern = DP_TRAINING_PATTERN_2;
>> +u32 training_pattern = DP_TRAINING_PATTERN_2;
>>   
>>  /*
>> - * Training Pattern 3 for HBR2 or 1.2 devices that support it.
>> - *
>>   * Intel platforms that support HBR2 also support TPS3. TPS3 support is
>>   * also mandatory for downstream devices that support HBR2.
>>   *
>> @@ -237,6 +236,18 @@ intel_dp_link_training_channel_equalization(struct 
>> intel_dp *intel_dp)
>>  else if (intel_dp->link_rate == 54)
>>  DRM_ERROR("5.4 Gbps link rate without HBR2/TPS3 support\n");
>>   
>> +return training_pattern;
>> +}
>> +
>> +static void
>> +intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
>> +{
>> +bool channel_eq = false;
>> +int tries, cr_tries;
>> +u32 training_pattern;
>> +
>> +training_pattern = intel_dp_training_pattern(intel_dp);
>> +
>>  /* channel equalization */
>>  if (!intel_dp_set_link_train(intel_dp,
>>   training_pattern |
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Intel-gfx] [PATCH] drm/i915: Skip DDI PLL selection for DSI

[Mika Kahola]

Skip DDI PLL selection if display type is DSI/MIPI.

Signed-off-by: Mika Kahola 
---
 drivers/gpu/drm/i915/intel_display.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index d7de2a5..5da98b2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9902,8 +9902,13 @@ static void broadwell_modeset_commit_cdclk(struct 
drm_atomic_state *old_state)
 static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
  struct intel_crtc_state *crtc_state)
 {
-   if (!intel_ddi_pll_select(crtc, crtc_state))
-   return -EINVAL;
+   struct intel_encoder *intel_encoder =
+   intel_ddi_get_crtc_new_encoder(crtc_state);
+
+   if (intel_encoder->type != INTEL_OUTPUT_DSI) {
+   if (!intel_ddi_pll_select(crtc, crtc_state))
+   return -EINVAL;
+   }
 
crtc->lowfreq_avail = false;
 
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH] list-workarounds: Extend the script to Mesa

[Damien Lespiau]

On Thu, Feb 04, 2016 at 06:14:02PM +, Kibey, Sameer wrote:
> Updated the list-workarounds script so that it
> can parse Mesa directory if provided. Moved the
> common code to a separate function to allow
> reuse for both kernel and mesa.
> 
> The new command line is:
> Usage: list-workarounds [options] path-to-kernel
>-k path-to-kernel -m path-to-mesa
> 
> The legacy usage is retained to avoid breaking
> backwards compatibility. New parameters -k and
> -m are added for the new behavior.
> 
> Either kernel or mesa or both paths can be specified.
> If path-to-mesa is invalid, error is reported.
> 
> Signed-off-by: Sameer Kibey 

Out of curiosity, how did you send the email? It doesn't seem to have
been sent with git send-email and so the patch isn't picked up by our
patchwork instance.

Out of the comments below, I guess the only serious one is allowing both
byt/vlv, but maybe mesa only uses one of the two? I wouldn't mind
landing the patch with that answered.

> ---
>  scripts/list-workarounds | 75 
> ++--
>  1 file changed, 54 insertions(+), 21 deletions(-)
> 
> diff --git a/scripts/list-workarounds b/scripts/list-workarounds
> index d11b6a9..0b63541 100755
> --- a/scripts/list-workarounds
> +++ b/scripts/list-workarounds
> @@ -18,7 +18,7 @@ def find_nth(haystack, needle, n):
>   return start
>  
>  valid_platforms = ('ctg', 'elk', 'ilk', 'snb', 'ivb', 'vlv', 'hsw', 'bdw',
> -'chv', 'skl', 'bxt')
> +'chv', 'skl', 'bxt', 'kbl', 'byt')

Do we really need both byt and vlv? that creates two different names for
the same platform, which sounds like a recipe to have the actual set of
W/As for this platform be the union of vlv and byt ones.

>  def parse_platforms(line, p):
>   l =  p.split(',')
>   for p in l:
> @@ -65,9 +65,15 @@ def execute(cmd):
>   return out, err
>  
>  def parse_options(args):
> - usage = "Usage: list-workarounds [options] path-to-kernel"
> + usage = "Usage: list-workarounds [options] path-to-kernel -k 
> path-to-kernel -m path-to-mesa"
>   parser = optparse.OptionParser(usage, version=1.0)

Quite frankly, I'd just remove the old behaviour.

> + parser.add_option("-k", "--kernel-path", dest="kernel_path", 
> default=None,
> +   help="path to kernel")
> +
> + parser.add_option("-m", "--mesa-path", dest="mesa_path", default=None,
> +   help="path to mesa")
> +
>   parser.add_option("-v", "--verbose", action="store_true",
> dest="verbose", default=False,
> help="be more verbose")
> @@ -76,30 +82,14 @@ def parse_options(args):
> help="List workarounds for the specified platform")
>  
>   (options, args) = parser.parse_args()
> -
>   return (options, args)
>  
> -if __name__ == '__main__':
> - (options, args) = parse_options(sys.argv[1:])
> - verbose = options.verbose
> -
> - if not len(args):
> - sys.stderr.write("error: A path to a kernel tree is required\n")
> - sys.exit(1)
> -
> - kernel_path = args[0]
> - kconfig = os.path.join(kernel_path, 'Kconfig')
> - if not os.path.isfile(kconfig):
> - sys.stderr.write("error: %s does not point to a kernel tree \n"
> -  % kernel_path)
> - sys.exit(1)
> -
> - i915_dir = os.path.join('drivers', 'gpu', 'drm', 'i915')
> +def print_workarounds(code_path, driver_dir):
>   olddir = os.getcwd()
> - os.chdir(kernel_path)
> + os.chdir(code_path)

project_root?

>   work_arounds, err = execute(['git', 'grep', '-n',
>'-e', 'W[aA][A-Z0-9][a-zA-Z0-9_]\+',
> -  i915_dir])
> +  driver_dir])
>   os.chdir(olddir)
>   if err:
>   print(err)
> @@ -111,3 +101,46 @@ if __name__ == '__main__':
>   print("%s: %s" % (wa, ', '.join(workarounds[wa])))
>   elif options.platform in workarounds[wa]:
>   print(wa)
> +
> +
> +if __name__ == '__main__':
> + (options, args) = parse_options(sys.argv)
> + verbose = options.verbose
> + kernel_path = None
> +
> + if not len(args) and options.kernel_path == None and options.mesa_path 
> == None:
> + sys.stderr.write("error: A path to either a kernel tree or Mesa 
> is required\n")
> + sys.exit(1)
> +
> + if len(args):
> + kernel_path = args[0]
> + elif options.kernel_path != None:
> + kernel_path = options.kernel_path
> +
> + if kernel_path != None:
> + # --- list Kernel workarounds if path is provided ---
> + kconfig = os.path.join(kernel_path, 'Kconfig')
> + if not os.path.isfile(kconfig):
> + sys.stderr.write("error: %s does not point to a kernel 
> tree \n"
> + 

[Intel-gfx] [PATCH v3] drm/i915/skl: Add missing SKL ids

[Michał Winiarski]

Used by production devices:
Intel(R) Iris Graphics 540 (Skylake GT3e)
Intel(R) Iris Graphics 550 (Skylake GT3e)

v2: More ids
v3: Less ids (GT1 got duplicated)

Cc: Mika Kuoppala 
Signed-off-by: Michał Winiarski 
Reviewed-by: Mika Kuoppala 
---
 include/drm/i915_pciids.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
index 9b48ac1..9094599 100644
--- a/include/drm/i915_pciids.h
+++ b/include/drm/i915_pciids.h
@@ -277,7 +277,9 @@
INTEL_VGA_DEVICE(0x191D, info)  /* WKS GT2 */
 
 #define INTEL_SKL_GT3_IDS(info) \
+   INTEL_VGA_DEVICE(0x1923, info), /* ULT GT3 */ \
INTEL_VGA_DEVICE(0x1926, info), /* ULT GT3 */ \
+   INTEL_VGA_DEVICE(0x1927, info), /* ULT GT3 */ \
INTEL_VGA_DEVICE(0x192B, info), /* Halo GT3 */ \
INTEL_VGA_DEVICE(0x192A, info)  /* SRV GT3 */
 
-- 
2.5.0

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

[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [RESEND,1/5] drm/i915: move VBT based TV presence check to intel_bios.c

[Patchwork]

== Summary ==

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

Test drv_module_reload_basic:
pass   -> DMESG-WARN (ilk-hp8440p)
Test gem_mmap_gtt:
Subgroup basic-small-bo:
pass   -> DMESG-WARN (ilk-hp8440p)
Test gem_sync:
Subgroup basic-bsd:
pass   -> DMESG-FAIL (ilk-hp8440p)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
dmesg-warn -> PASS   (ilk-hp8440p) UNSTABLE
Test pm_rpm:
Subgroup basic-pci-d3-state:
dmesg-warn -> PASS   (skl-i5k-2)

bdw-nuci7total:161  pass:152  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultratotal:164  pass:152  dwarn:0   dfail:0   fail:0   skip:12 
byt-nuc  total:164  pass:141  dwarn:0   dfail:0   fail:0   skip:23 
hsw-brixbox  total:164  pass:151  dwarn:0   dfail:0   fail:0   skip:13 
hsw-gt2  total:164  pass:154  dwarn:0   dfail:0   fail:0   skip:10 
ilk-hp8440p  total:164  pass:113  dwarn:2   dfail:1   fail:0   skip:48 
ivb-t430stotal:164  pass:150  dwarn:0   dfail:0   fail:0   skip:14 
skl-i5k-2total:164  pass:149  dwarn:1   dfail:0   fail:0   skip:14 
skl-i7k-2total:164  pass:149  dwarn:1   dfail:0   fail:0   skip:14 
snb-dellxps  total:164  pass:142  dwarn:0   dfail:0   fail:0   skip:22 
snb-x220ttotal:164  pass:142  dwarn:0   dfail:0   fail:1   skip:21 

Results at /archive/results/CI_IGT_test/Patchwork_1368/

57e229193395068adcb34c5266d54194e652869f drm-intel-nightly: 
2016y-02m-04d-18h-38m-55s UTC integration manifest
31400d242652b6705a98f9539c685daf4f8d46c9 drm/i915/panel: setup pwm backlight 
based on connector type
fb6c1a1a33ed8c55450251bace69f42b98f23e9b drm/i915: move VBT based DSI presence 
check to intel_bios.c
dbc3f4f96e24ae58849c4775f8c1cae3c0d70065 drm/i915: move VBT based eDP port 
check to intel_bios.c
3389e0bfcd2611ac99db098fddb936f9eb6f9259 drm/i915: move VBT based LVDS presence 
check to intel_bios.c
4a1453567944519983f85e212a8eb93084834656 drm/i915: move VBT based TV presence 
check to intel_bios.c

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

[Intel-gfx] [PATCH 0/2] DPCD Backlight Control

[Yetunde Adebisi]

These patches add support for Backlight Control using DPCD registers on eDP 
displays.

- Patch 1 adds macro for DPCD registers capability size to drm_dp_helper.h
A copy of this patch has also been sent to dri-devel list.

- Patch 2 Implements functionaly for DPCD Backlight Control 


Yetunde Adebisi (2):
  drm/dp: Add definition for Display Control DPCD Registers capability
size
  drm/i915: Add Backlight Control using DPCD for eDP connectors (v6)

 drivers/gpu/drm/i915/Makefile |   1 +
 drivers/gpu/drm/i915/intel_dp.c   |  17 ++-
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 170 ++
 drivers/gpu/drm/i915/intel_drv.h  |   6 +
 drivers/gpu/drm/i915/intel_panel.c|   4 +
 include/drm/drm_dp_helper.h   |   1 +
 6 files changed, 193 insertions(+), 6 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_dp_aux_backlight.c

-- 
1.9.3

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

Re: [Intel-gfx] [PATCH 1/2] drm/i915/dp: abstract training pattern selection

[Thulasimani, Sivakumar]

Reviewed-by: Sivakumar Thulasimani 

On 2/5/2016 3:46 PM, Jani Nikula wrote:

Make it cleaner to add more checks in the function. No functional
changes.

Cc: Ander Conselvan de Oliveira 
Cc: Sivakumar Thulasimani 
Cc: drm-intel-fi...@lists.freedesktop.org # dependency on the next patch
Signed-off-by: Jani Nikula 
---
  drivers/gpu/drm/i915/intel_dp_link_training.c | 25 ++---
  1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c 
b/drivers/gpu/drm/i915/intel_dp_link_training.c
index 7938e0bf..83e667b92fda 100644
--- a/drivers/gpu/drm/i915/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
@@ -215,16 +215,15 @@ intel_dp_link_training_clock_recovery(struct intel_dp 
*intel_dp)
}
  }
  
-static void

-intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
+/*
+ * Pick training pattern for channel equalization. Training Pattern 3 for HBR2
+ * or 1.2 devices that support it, Training Pattern 2 otherwise.
+ */
+static u32 intel_dp_training_pattern(struct intel_dp *intel_dp)
  {
-   bool channel_eq = false;
-   int tries, cr_tries;
-   uint32_t training_pattern = DP_TRAINING_PATTERN_2;
+   u32 training_pattern = DP_TRAINING_PATTERN_2;
  
  	/*

-* Training Pattern 3 for HBR2 or 1.2 devices that support it.
-*
 * Intel platforms that support HBR2 also support TPS3. TPS3 support is
 * also mandatory for downstream devices that support HBR2.
 *
@@ -237,6 +236,18 @@ intel_dp_link_training_channel_equalization(struct 
intel_dp *intel_dp)
else if (intel_dp->link_rate == 54)
DRM_ERROR("5.4 Gbps link rate without HBR2/TPS3 support\n");
  
+	return training_pattern;

+}
+
+static void
+intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
+{
+   bool channel_eq = false;
+   int tries, cr_tries;
+   u32 training_pattern;
+
+   training_pattern = intel_dp_training_pattern(intel_dp);
+
/* channel equalization */
if (!intel_dp_set_link_train(intel_dp,
 training_pattern |


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

[Intel-gfx] [PATCH 1/2] drm/dp: Add definition for Display Control DPCD Registers capability size

[Yetunde Adebisi]

This is used when reading Display Control capability Registers on the sink
device.

cc: Jani Nikula 
Signed-off-by: Yetunde Adebisi 
---
 include/drm/drm_dp_helper.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 1252108..92d9a52 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -621,6 +621,7 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 
link_status[DP_LINK_STATUS_SI
 #define DP_BRANCH_OUI_HEADER_SIZE  0xc
 #define DP_RECEIVER_CAP_SIZE   0xf
 #define EDP_PSR_RECEIVER_CAP_SIZE  2
+#define EDP_DISPLAY_CTL_CAP_SIZE   3
 
 void drm_dp_link_train_clock_recovery_delay(const u8 
dpcd[DP_RECEIVER_CAP_SIZE]);
 void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]);
-- 
1.9.3

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

[Intel-gfx] ✓ Fi.CI.BAT: success for DPCD Backlight Control (rev3)

[Patchwork]

== Summary ==

Series 1864v3 DPCD Backlight Control
http://patchwork.freedesktop.org/api/1.0/series/1864/revisions/3/mbox/

Test kms_flip:
Subgroup basic-flip-vs-modeset:
pass   -> DMESG-WARN (ilk-hp8440p) UNSTABLE

bdw-nuci7total:161  pass:152  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultratotal:164  pass:152  dwarn:0   dfail:0   fail:0   skip:12 
bsw-nuc-2total:164  pass:136  dwarn:0   dfail:0   fail:0   skip:28 
byt-nuc  total:164  pass:141  dwarn:0   dfail:0   fail:0   skip:23 
hsw-brixbox  total:164  pass:151  dwarn:0   dfail:0   fail:0   skip:13 
hsw-gt2  total:164  pass:154  dwarn:0   dfail:0   fail:0   skip:10 
ilk-hp8440p  total:164  pass:114  dwarn:2   dfail:0   fail:0   skip:48 
skl-i7k-2total:164  pass:149  dwarn:1   dfail:0   fail:0   skip:14 
snb-dellxps  total:164  pass:142  dwarn:0   dfail:0   fail:0   skip:22 
snb-x220ttotal:164  pass:142  dwarn:0   dfail:0   fail:1   skip:21 

Results at /archive/results/CI_IGT_test/Patchwork_1370/

57e229193395068adcb34c5266d54194e652869f drm-intel-nightly: 
2016y-02m-04d-18h-38m-55s UTC integration manifest
d492a5ab6749af90c6cab465184a10ba751fa63a drm/i915: Add Backlight Control using 
DPCD for eDP connectors (v6)
b1ff81d3779295705ac8b86088e616c5a43e1d85 drm/dp: Add definition for Display 
Control DPCD Registers capability size

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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/skl: Add missing SKL GT3 id (rev3)

[Patchwork]

== Summary ==

Series 2919v3 drm/i915/skl: Add missing SKL GT3 id
http://patchwork.freedesktop.org/api/1.0/series/2919/revisions/3/mbox/

Test kms_flip:
Subgroup basic-flip-vs-modeset:
pass   -> DMESG-WARN (ilk-hp8440p) UNSTABLE
Test kms_force_connector_basic:
Subgroup force-edid:
skip   -> PASS   (ilk-hp8440p)
Test kms_pipe_crc_basic:
Subgroup nonblocking-crc-pipe-b-frame-sequence:
dmesg-warn -> PASS   (ilk-hp8440p)

bdw-nuci7total:161  pass:152  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultratotal:164  pass:152  dwarn:0   dfail:0   fail:0   skip:12 
bsw-nuc-2total:164  pass:136  dwarn:0   dfail:0   fail:0   skip:28 
byt-nuc  total:164  pass:141  dwarn:0   dfail:0   fail:0   skip:23 
hsw-brixbox  total:164  pass:151  dwarn:0   dfail:0   fail:0   skip:13 
hsw-gt2  total:164  pass:154  dwarn:0   dfail:0   fail:0   skip:10 
ilk-hp8440p  total:164  pass:115  dwarn:1   dfail:0   fail:0   skip:48 
snb-dellxps  total:164  pass:142  dwarn:0   dfail:0   fail:0   skip:22 

Results at /archive/results/CI_IGT_test/Patchwork_1371/

ba94996e08e55e342fb630c259d08e1b8d0f9c03 drm-intel-nightly: 
2016y-02m-05d-12h-48m-46s UTC integration manifest
1a29ddbf894cf3f1e05e752cf6f2bf8902f09946 drm/i915/skl: Add missing SKL ids

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

Re: [Intel-gfx] [RFC 02/29] drm/i915: Introduce host graphics memory balloon for gvt

[Joonas Lahtinen]

Hi,

On pe, 2016-02-05 at 18:03 +0800, Zhiyuan Lv wrote:
> Hi Joonas,
> 
> Thanks much for the review! We will incorporate those review comments!
> 
> Meanwhile, is it good enough to do the host ballooning like below? The
> pros is that it is very simple, especially consider that guest
> ballooning logic has been there. Thanks!
> 

I think slicing it down like that is fine. Is there going to be real
ballooning happening, as in, is the available memory dynamically going
to change during runtime, or just be fixed size since the beginning?

Regards, Joonas

> Regards,
> -Zhiyuan
> 
> On Thu, Feb 04, 2016 at 01:27:08PM +0200, Joonas Lahtinen wrote:
> > Hi,
> > 
> > On to, 2016-01-28 at 18:21 +0800, Zhi Wang wrote:
> > > From: Bing Niu 
> > > 
> > > This patch introduces host graphics memory ballon when GVT-g is enabled.
> > > 
> > > As under GVT-g, i915 only owned limited graphics resources, others are
> > > managed by GVT-g resource allocator and kept for other vGPUs.
> > > 
> > > For graphics memory space partition, a typical layout looks like:
> > > 
> > > +---+---+--+---+
> > > > * Host |   *GVT-g Resource |* Host|   *GVT-g Resource |
> > > > Owned |   Allocator Managed   | Owned|   Allocator Managed   |
> > > >   |   |  |   |
> > > +---+---+--+---+---+
> > > >   |   |   |   |  |   |   |   |
> > > > i915  | vm 1  | vm 2  | vm 3  | i915 | vm 1  | vm 2  | vm 3  |
> > > >   |   |   |   |  |   |   |   |
> > > +---+---+---+--+---+---+---+
> > > >   Aperture|Hidden|
> > > +---+--+
> > > >   GGTT memory space  |
> > > +--+
> > > 
> > > Similar with fence registers partition:
> > > 
> > >  +-- +---+
> > >  | * Host|GVT-g Resource |
> > >  | Owned |   Allocator Managed   +
> > >  0   |   31
> > >  +---+---+---+
> > >  |   |   |   |   |
> > >  | i915  | vm 1  | vm 2  | vm 3  |
> > >  |   |   |   |   |
> > >  +---+---+---+---+
> > > 
> > > i915 host will read the amount of allocated resources via GVT-g kernel 
> > > parameters.
> > > 
> > > Signed-off-by: Bing Niu 
> > > Signed-off-by: Zhi Wang 
> > > ---
> > >  drivers/gpu/drm/i915/gvt/params.h   |  3 +++
> > >  drivers/gpu/drm/i915/i915_gem.c |  3 +++
> > >  drivers/gpu/drm/i915/i915_gem_gtt.c |  4 ++--
> > >  drivers/gpu/drm/i915/i915_vgpu.c| 16 
> > >  drivers/gpu/drm/i915/i915_vgpu.h|  1 +
> > >  5 files changed, 21 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gvt/params.h 
> > > b/drivers/gpu/drm/i915/gvt/params.h
> > > index d2955b9..0656a98 100644
> > > --- a/drivers/gpu/drm/i915/gvt/params.h
> > > +++ b/drivers/gpu/drm/i915/gvt/params.h
> > > @@ -27,6 +27,9 @@
> > >  struct gvt_kernel_params {
> > >   bool enable;
> > >   int debug;
> > > + int dom0_low_gm_sz;
> > > + int dom0_high_gm_sz;
> > > + int dom0_fence_sz;

Some comment on the unit would be nice here, as there is a shift with
<< 20 later on.

Is "unsigned long" type and count in pages not acceptable? I think
that's the assumption one is going to make.

> > >  };
> > >  
> > >  extern struct gvt_kernel_params gvt;
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c 
> > > b/drivers/gpu/drm/i915/i915_gem.c
> > > index 799a53a..e916e43 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -5080,6 +5080,9 @@ i915_gem_load(struct drm_device *dev)
> > >   else
> > >   dev_priv->num_fence_regs = 8;
> > >  
> > > + if(intel_gvt_host_active(dev))
> > 
> > Space between "if" and "("
> 
> Thanks! Will correct it.
> 
> > 
> > > + dev_priv->num_fence_regs = gvt.dom0_fence_sz;
> > > +
> > >   if (intel_vgpu_active(dev))
> > >   dev_priv->num_fence_regs =
> > >   I915_READ(vgtif_reg(avail_rs.fence_num));
> > 
> > I'd like to see the above as "else if" construct just like you have
> > below with the intel_vgt_balloon(). Could even do
> > 
> > if (gvt_host)  {
> > ...
> > } else if (vgpu_active) {
> > ...
> > } else {
> > per machine detection
> > }
> > 
> > Right?
> 
> That looks better!
> 
> > 
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> > > b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > index 7377b67..0540de2 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > @@ -2713,7 +2713,7 @@ static int 

[Intel-gfx] [PATCH:xf86-intel-video] Add NULL checking for drawable in sna_dri2_flip_event

[Lim Siew Hoon]

The last flip complete signal may happen after the
sna_dri2_destroy_window function has been called. This
leads to calling frame_swap_complete with a null flip
drawable. So check for that and handle accordingly.

Signed-off-by: Lim Siew Hoon 
Reviewed-by: Bob Paauwe 
Signed-off-by: Matt Roper 
---
 src/sna/sna_dri2.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/sna/sna_dri2.c b/src/sna/sna_dri2.c
index 33cf3d9..764a824 100644
--- a/src/sna/sna_dri2.c
+++ b/src/sna/sna_dri2.c
@@ -2874,6 +2874,10 @@ static void sna_dri2_flip_event(struct sna_dri2_event 
*flip)
case FLIP_THROTTLE:
if (flip->signal) {
DBG(("%s: triple buffer swap complete, unblocking 
client\n", __FUNCTION__));
+   if(flip->draw == NULL) {
+   sna_dri2_event_free(flip);
+   break;
+   }
frame_swap_complete(flip, DRI2_FLIP_COMPLETE);
}
case FLIP_COMPLETE:
-- 
2.1.0

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

[Intel-gfx] [PATCH 1/2] drm/i915/dp: abstract training pattern selection

[Jani Nikula]

Make it cleaner to add more checks in the function. No functional
changes.

Cc: Ander Conselvan de Oliveira 
Cc: Sivakumar Thulasimani 
Cc: drm-intel-fi...@lists.freedesktop.org # dependency on the next patch
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_dp_link_training.c | 25 ++---
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c 
b/drivers/gpu/drm/i915/intel_dp_link_training.c
index 7938e0bf..83e667b92fda 100644
--- a/drivers/gpu/drm/i915/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
@@ -215,16 +215,15 @@ intel_dp_link_training_clock_recovery(struct intel_dp 
*intel_dp)
}
 }
 
-static void
-intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
+/*
+ * Pick training pattern for channel equalization. Training Pattern 3 for HBR2
+ * or 1.2 devices that support it, Training Pattern 2 otherwise.
+ */
+static u32 intel_dp_training_pattern(struct intel_dp *intel_dp)
 {
-   bool channel_eq = false;
-   int tries, cr_tries;
-   uint32_t training_pattern = DP_TRAINING_PATTERN_2;
+   u32 training_pattern = DP_TRAINING_PATTERN_2;
 
/*
-* Training Pattern 3 for HBR2 or 1.2 devices that support it.
-*
 * Intel platforms that support HBR2 also support TPS3. TPS3 support is
 * also mandatory for downstream devices that support HBR2.
 *
@@ -237,6 +236,18 @@ intel_dp_link_training_channel_equalization(struct 
intel_dp *intel_dp)
else if (intel_dp->link_rate == 54)
DRM_ERROR("5.4 Gbps link rate without HBR2/TPS3 support\n");
 
+   return training_pattern;
+}
+
+static void
+intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
+{
+   bool channel_eq = false;
+   int tries, cr_tries;
+   u32 training_pattern;
+
+   training_pattern = intel_dp_training_pattern(intel_dp);
+
/* channel equalization */
if (!intel_dp_set_link_train(intel_dp,
 training_pattern |
-- 
2.1.4

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

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/dp: abstract training pattern selection

[Patchwork]

== Summary ==

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

Test kms_flip:
Subgroup basic-flip-vs-dpms:
dmesg-warn -> PASS   (ilk-hp8440p) UNSTABLE
Test pm_rpm:
Subgroup basic-pci-d3-state:
dmesg-warn -> PASS   (skl-i5k-2)

bdw-nuci7total:161  pass:152  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultratotal:164  pass:152  dwarn:0   dfail:0   fail:0   skip:12 
bsw-nuc-2total:164  pass:136  dwarn:0   dfail:0   fail:0   skip:28 
byt-nuc  total:164  pass:141  dwarn:0   dfail:0   fail:0   skip:23 
hsw-brixbox  total:164  pass:151  dwarn:0   dfail:0   fail:0   skip:13 
hsw-gt2  total:164  pass:154  dwarn:0   dfail:0   fail:0   skip:10 
ilk-hp8440p  total:164  pass:116  dwarn:0   dfail:0   fail:0   skip:48 
ivb-t430stotal:164  pass:150  dwarn:0   dfail:0   fail:0   skip:14 
skl-i5k-2total:164  pass:149  dwarn:1   dfail:0   fail:0   skip:14 
snb-dellxps  total:164  pass:142  dwarn:0   dfail:0   fail:0   skip:22 
snb-x220ttotal:164  pass:142  dwarn:0   dfail:0   fail:1   skip:21 

Results at /archive/results/CI_IGT_test/Patchwork_1367/

57e229193395068adcb34c5266d54194e652869f drm-intel-nightly: 
2016y-02m-04d-18h-38m-55s UTC integration manifest
40076067dd294abb76241fbdd4e5ba7261a3df41 drm/i915/dp: reduce missing TPS3 
support errors to debug logging
173ef8cfe90938cdbd60724deead3989885275e0 drm/i915/dp: abstract training pattern 
selection

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

Re: [Intel-gfx] [RFC 02/29] drm/i915: Introduce host graphics memory balloon for gvt

[Zhiyuan Lv]

Hi Joonas,

On Fri, Feb 05, 2016 at 03:40:49PM +0200, Joonas Lahtinen wrote:
> Hi,
> 
> On pe, 2016-02-05 at 18:03 +0800, Zhiyuan Lv wrote:
> > Hi Joonas,
> > 
> > Thanks much for the review! We will incorporate those review comments!
> > 
> > Meanwhile, is it good enough to do the host ballooning like below? The
> > pros is that it is very simple, especially consider that guest
> > ballooning logic has been there. Thanks!
> > 
> 
> I think slicing it down like that is fine. Is there going to be real
> ballooning happening, as in, is the available memory dynamically going
> to change during runtime, or just be fixed size since the beginning?

In our current design, once the gvt is enabled, it is fixed size of graphics
memory for host since beginning. The reserved memory is all for virtual
machines.

We ever considered to support dynamically enabling/disabling gvt, that host
i915 could boot without gvt enabled, and can use all the hardware resources.
When there is need to creat VM, we can reload i915 driver to reserve the
resources. Does that sound good? Thanks!

Regards,
-Zhiyuan

> 
> Regards, Joonas
> 
> > Regards,
> > -Zhiyuan
> > 
> > On Thu, Feb 04, 2016 at 01:27:08PM +0200, Joonas Lahtinen wrote:
> > > Hi,
> > > 
> > > On to, 2016-01-28 at 18:21 +0800, Zhi Wang wrote:
> > > > From: Bing Niu 
> > > > 
> > > > This patch introduces host graphics memory ballon when GVT-g is enabled.
> > > > 
> > > > As under GVT-g, i915 only owned limited graphics resources, others are
> > > > managed by GVT-g resource allocator and kept for other vGPUs.
> > > > 
> > > > For graphics memory space partition, a typical layout looks like:
> > > > 
> > > > +---+---+--+---+
> > > > > * Host |   *GVT-g Resource |* Host|   *GVT-g Resource |
> > > > > Owned |   Allocator Managed   | Owned|   Allocator Managed   |
> > > > >   |   |  |   |
> > > > +---+---+--+---+---+
> > > > >   |   |   |   |  |   |   |   |
> > > > > i915  | vm 1  | vm 2  | vm 3  | i915 | vm 1  | vm 2  | vm 3  |
> > > > >   |   |   |   |  |   |   |   |
> > > > +---+---+---+--+---+---+---+
> > > > >   Aperture|Hidden|
> > > > +---+--+
> > > > >   GGTT memory space  |
> > > > +--+
> > > > 
> > > > Similar with fence registers partition:
> > > > 
> > > >  +-- +---+
> > > >  | * Host|GVT-g Resource |
> > > >  | Owned |   Allocator Managed   +
> > > >  0   |   31
> > > >  +---+---+---+
> > > >  |   |   |   |   |
> > > >  | i915  | vm 1  | vm 2  | vm 3  |
> > > >  |   |   |   |   |
> > > >  +---+---+---+---+
> > > > 
> > > > i915 host will read the amount of allocated resources via GVT-g kernel 
> > > > parameters.
> > > > 
> > > > Signed-off-by: Bing Niu 
> > > > Signed-off-by: Zhi Wang 
> > > > ---
> > > >  drivers/gpu/drm/i915/gvt/params.h   |  3 +++
> > > >  drivers/gpu/drm/i915/i915_gem.c |  3 +++
> > > >  drivers/gpu/drm/i915/i915_gem_gtt.c |  4 ++--
> > > >  drivers/gpu/drm/i915/i915_vgpu.c| 16 
> > > >  drivers/gpu/drm/i915/i915_vgpu.h|  1 +
> > > >  5 files changed, 21 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/gvt/params.h 
> > > > b/drivers/gpu/drm/i915/gvt/params.h
> > > > index d2955b9..0656a98 100644
> > > > --- a/drivers/gpu/drm/i915/gvt/params.h
> > > > +++ b/drivers/gpu/drm/i915/gvt/params.h
> > > > @@ -27,6 +27,9 @@
> > > >  struct gvt_kernel_params {
> > > >     bool enable;
> > > >     int debug;
> > > > +   int dom0_low_gm_sz;
> > > > +   int dom0_high_gm_sz;
> > > > +   int dom0_fence_sz;
> 
> Some comment on the unit would be nice here, as there is a shift with
> << 20 later on.
> 
> Is "unsigned long" type and count in pages not acceptable? I think
> that's the assumption one is going to make.
> 
> > > >  };
> > > >  
> > > >  extern struct gvt_kernel_params gvt;
> > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c 
> > > > b/drivers/gpu/drm/i915/i915_gem.c
> > > > index 799a53a..e916e43 100644
> > > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > > @@ -5080,6 +5080,9 @@ i915_gem_load(struct drm_device *dev)
> > > >     else
> > > >     dev_priv->num_fence_regs = 8;
> > > >  
> > > > +   if(intel_gvt_host_active(dev))
> > > 
> > > Space between "if" and "("
> > 
> > Thanks! Will correct it.
> > 
> > > 
> > > > +   dev_priv->num_fence_regs = gvt.dom0_fence_sz;
> > 

Re: [Intel-gfx] [v2 1/6] drm/i915/skl+: Use plane size for relative data rate calculation

[Matt Roper]

On Wed, Jan 27, 2016 at 09:39:58PM +0530, Shobhit Kumar wrote:
> From: "Kumar, Mahesh" 
> 
> Use plane size for relative data rate calculation. don't always use
> pipe source width & height.
> adjust height & width according to rotation.
> use plane size for watermark calculations also.
> 
> v2: Address Matt's comments.
> Use intel_plane_state->visible to avoid divide-by-zero error.
> Where FB was present but not visible so causing total data rate to
> be zero, hence divide-by-zero.
> 
> Cc: matthew.d.ro...@intel.com
> Signed-off-by: Kumar, Mahesh 

Reviewed-by: Matt Roper 

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 42 
> ++---
>  1 file changed, 27 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 20bf854..d55e5d0 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2868,25 +2868,28 @@ skl_plane_relative_data_rate(const struct 
> intel_crtc_state *cstate,
>const struct drm_plane_state *pstate,
>int y)
>  {
> - struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
> + struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
>   struct drm_framebuffer *fb = pstate->fb;
> + uint32_t width = 0, height = 0;
> +
> + width = drm_rect_width(_pstate->src) >> 16;
> + height = drm_rect_height(_pstate->src) >> 16;
> +
> + if (intel_rotation_90_or_270(pstate->rotation))
> + swap(width, height);
>  
>   /* for planar format */
>   if (fb->pixel_format == DRM_FORMAT_NV12) {
>   if (y)  /* y-plane data rate */
> - return intel_crtc->config->pipe_src_w *
> - intel_crtc->config->pipe_src_h *
> + return width * height *
>   drm_format_plane_cpp(fb->pixel_format, 0);
>   else/* uv-plane data rate */
> - return (intel_crtc->config->pipe_src_w/2) *
> - (intel_crtc->config->pipe_src_h/2) *
> + return (width / 2) * (height / 2) *
>   drm_format_plane_cpp(fb->pixel_format, 1);
>   }
>  
>   /* for packed formats */
> - return intel_crtc->config->pipe_src_w *
> - intel_crtc->config->pipe_src_h *
> - drm_format_plane_cpp(fb->pixel_format, 0);
> + return width * height * drm_format_plane_cpp(fb->pixel_format, 0);
>  }
>  
>  /*
> @@ -2905,9 +2908,8 @@ skl_get_total_relative_data_rate(const struct 
> intel_crtc_state *cstate)
>   for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
>   const struct drm_plane_state *pstate = intel_plane->base.state;
>  
> - if (pstate->fb == NULL)
> + if (!to_intel_plane_state(pstate)->visible)
>   continue;
> -
>   if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)
>   continue;
>  
> @@ -2965,8 +2967,9 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>   struct drm_framebuffer *fb = plane->state->fb;
>   int id = skl_wm_plane_id(intel_plane);
>  
> - if (fb == NULL)
> + if (!to_intel_plane_state(plane->state)->visible)
>   continue;
> +
>   if (plane->type == DRM_PLANE_TYPE_CURSOR)
>   continue;
>  
> @@ -2992,7 +2995,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>   uint16_t plane_blocks, y_plane_blocks = 0;
>   int id = skl_wm_plane_id(intel_plane);
>  
> - if (pstate->fb == NULL)
> + if (!to_intel_plane_state(pstate)->visible)
>   continue;
>   if (plane->type == DRM_PLANE_TYPE_CURSOR)
>   continue;
> @@ -3116,28 +3119,37 @@ static bool skl_compute_plane_wm(const struct 
> drm_i915_private *dev_priv,
>  {
>   struct drm_plane *plane = _plane->base;
>   struct drm_framebuffer *fb = plane->state->fb;
> + struct intel_plane_state *intel_pstate =
> + to_intel_plane_state(plane->state);
>   uint32_t latency = dev_priv->wm.skl_latency[level];
>   uint32_t method1, method2;
>   uint32_t plane_bytes_per_line, plane_blocks_per_line;
>   uint32_t res_blocks, res_lines;
>   uint32_t selected_result;
>   uint8_t bytes_per_pixel;
> + uint32_t width = 0, height = 0;
>  
> - if (latency == 0 || !cstate->base.active || !fb)
> + if (latency == 0 || !cstate->base.active || !intel_pstate->visible)
>   return false;
>  
> + width = drm_rect_width(_pstate->src) >> 16;
> + height = drm_rect_height(_pstate->src) >> 16;
> +
> + if 

Re: [Intel-gfx] [v2 2/6] drm/i915/skl+: calculate ddb minimum allocation

[Matt Roper]

On Wed, Jan 27, 2016 at 09:39:59PM +0530, Shobhit Kumar wrote:
> From: "Kumar, Mahesh" 
> 
> don't always use 8 ddb as minimum, instead calculate using proper
> algorithm.
> 
> v2: optimizations as per Matt's comments.
> 
> Cc: matthew.d.ro...@intel.com
> Signed-off-by: Kumar, Mahesh 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 50 
> ++---
>  1 file changed, 47 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d55e5d0..708f329 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2928,6 +2928,51 @@ skl_get_total_relative_data_rate(const struct 
> intel_crtc_state *cstate)
>   return total_data_rate;
>  }
>  
> +static uint16_t
> +skl_ddb_min_alloc(const struct intel_crtc *crtc,
> + const struct drm_plane *plane, int y)
> +{
> + struct drm_framebuffer *fb = plane->state->fb;
> + struct intel_plane_state *pstate = to_intel_plane_state(plane->state);
> + uint32_t src_w, src_h;
> + uint32_t min_scanlines = 8;
> + uint8_t bytes_per_pixel;
> +
> + /* For packed formats, no y-plane, return 0 */
> + if (y && !fb && !(fb->pixel_format == DRM_FORMAT_NV12))

I think you meant

if (!fb || (y && fb->pixel_format != DRM_FORMAT_NV12))

right?

> + return 0;
> +
> + /* For Non Y-tile return 8-blocks */
> + if (!(fb->modifier[0] == I915_FORMAT_MOD_Y_TILED) &&
> + !(fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED))
> + return 8;

Minor nitpick, but it might be more readable to use != instead of
pulling the ! outside of the comparison.  On my first quick read I
missed the inversions.


Matt

> +
> + src_w = drm_rect_width(>src) >> 16;
> + src_h = drm_rect_height(>src) >> 16;
> +
> + if (intel_rotation_90_or_270(plane->state->rotation))
> + swap(src_w, src_h);
> +
> + bytes_per_pixel = y ? drm_format_plane_cpp(fb->pixel_format, 0) :
> + drm_format_plane_cpp(fb->pixel_format, 1);
> +
> + if (intel_rotation_90_or_270(plane->state->rotation)) {
> + switch (bytes_per_pixel) {
> + case 1:
> + min_scanlines = 32;
> + break;
> + case 2:
> + min_scanlines = 16;
> + break;
> + case 8:
> + WARN(1, "Unsupported pixel depth for rotation");
> + }
> + }
> +
> + return DIV_ROUND_UP((4 * src_w / (y ? 1 : 2) * bytes_per_pixel), 512) *
> + min_scanlines/4 + 3;
> +}
> +
>  static void
>  skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
> struct skl_ddb_allocation *ddb /* out */)
> @@ -2964,7 +3009,6 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>   /* 1. Allocate the mininum required blocks for each active plane */
>   for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
>   struct drm_plane *plane = _plane->base;
> - struct drm_framebuffer *fb = plane->state->fb;
>   int id = skl_wm_plane_id(intel_plane);
>  
>   if (!to_intel_plane_state(plane->state)->visible)
> @@ -2973,9 +3017,9 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>   if (plane->type == DRM_PLANE_TYPE_CURSOR)
>   continue;
>  
> - minimum[id] = 8;
> + minimum[id] = skl_ddb_min_alloc(intel_crtc, plane, 0);
>   alloc_size -= minimum[id];
> - y_minimum[id] = (fb->pixel_format == DRM_FORMAT_NV12) ? 8 : 0;
> + y_minimum[id] = skl_ddb_min_alloc(intel_crtc, plane, 1);
>   alloc_size -= y_minimum[id];
>   }
>  
> -- 
> 2.5.0
> 

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

[Intel-gfx] [PATCH] drm/i915: Check for get_pages instead of shmem (filp)

[Ben Widawsky]

This behavior of checking for a shmem backed GEM object was introduced here:
commit 4c914c0c7c787b8f730128a8cdcca9c50b0784ab
Author: Brad Volkin 
Date:   Tue Feb 18 10:15:45 2014 -0800

drm/i915: Refactor shmem pread setup

It is possible for an object to not be a shmem backed GEM object (for example
userptr objects). An example of how we hit this failure can be found through
copy_batch() in the command parser because we allocate a userptr object for the
batch which contains privileged instructions. Userptr calls
drm_gem_private_object_init() which explicitly sets the filp to none.

It is equally feasible to simply remove the check altogether. You'll probably
oops with get_pages somewhere, but that's okay IMO because this condition
should be a driver bug, and not trigger-able by userspace. On this note, the
function name could probably benefit from a change, but whatever.

NOTE: I manually retyped this from a test machine. So I haven't even compiled
this exact patch.

Cc: Chris Wilson 
Cc: Kristian Høgsberg 
Cc: Jordan Justen 
Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 66b1705..a198928 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -489,7 +489,7 @@ int i915_gem_obj_prepare_shmem_read(struct 
drm_i915_gem_object *obj,
 
*needs_clflush = 0;
 
-   if (!obj->base.filp)
+   if (!obj->ops->get_pages)
return -EINVAL;
 
if (!(obj->base.read_domains & I915_GEM_DOMAIN_CPU)) {
-- 
2.7.0

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

Re: [Intel-gfx] [PATCH v2 i-g-t] igt/list-workarounds: Extend the script to Mesa

[Damien Lespiau]

On Fri, Feb 05, 2016 at 01:55:19PM -0800, Sameer Kibey wrote:
> Updated the list-workarounds script so that it
> can parse Mesa directory if provided. Moved the
> common code to a separate function to allow
> reuse for both kernel and mesa.
> 
> The new command line is:
> Usage: list-workarounds [options] path-to-kernel
>-k path-to-kernel -m path-to-mesa
> 
> The legacy usage is retained to avoid breaking
> backwards compatibility. New parameters -k and
> -m are added for the new behavior.
> 
> Either kernel or mesa or both paths can be specified.
> If path-to-mesa is invalid, error is reported.
> 
> Signed-off-by: Sameer Kibey 

Pushed thanks for the patch.

-- 
Damien

> ---
>  scripts/list-workarounds | 74 
> ++--
>  1 file changed, 53 insertions(+), 21 deletions(-)
> 
> diff --git a/scripts/list-workarounds b/scripts/list-workarounds
> index d11b6a9..8b41ae5 100755
> --- a/scripts/list-workarounds
> +++ b/scripts/list-workarounds
> @@ -18,7 +18,7 @@ def find_nth(haystack, needle, n):
>   return start
>  
>  valid_platforms = ('ctg', 'elk', 'ilk', 'snb', 'ivb', 'vlv', 'hsw', 'bdw',
> -'chv', 'skl', 'bxt')
> +'chv', 'skl', 'bxt', 'kbl')
>  def parse_platforms(line, p):
>   l =  p.split(',')
>   for p in l:
> @@ -65,9 +65,15 @@ def execute(cmd):
>   return out, err
>  
>  def parse_options(args):
> - usage = "Usage: list-workarounds [options] path-to-kernel"
> + usage = "Usage: list-workarounds [options] path-to-kernel -k 
> path-to-kernel -m path-to-mesa"
>   parser = optparse.OptionParser(usage, version=1.0)
>  
> + parser.add_option("-k", "--kernel-path", dest="kernel_path", 
> default=None,
> +   help="path to kernel")
> +
> + parser.add_option("-m", "--mesa-path", dest="mesa_path", default=None,
> +   help="path to mesa")
> +
>   parser.add_option("-v", "--verbose", action="store_true",
> dest="verbose", default=False,
> help="be more verbose")
> @@ -76,38 +82,64 @@ def parse_options(args):
> help="List workarounds for the specified platform")
>  
>   (options, args) = parser.parse_args()
> -
>   return (options, args)
>  
> -if __name__ == '__main__':
> - (options, args) = parse_options(sys.argv[1:])
> - verbose = options.verbose
> -
> - if not len(args):
> - sys.stderr.write("error: A path to a kernel tree is required\n")
> - sys.exit(1)
> -
> - kernel_path = args[0]
> - kconfig = os.path.join(kernel_path, 'Kconfig')
> - if not os.path.isfile(kconfig):
> - sys.stderr.write("error: %s does not point to a kernel tree \n"
> -  % kernel_path)
> - sys.exit(1)
> -
> - i915_dir = os.path.join('drivers', 'gpu', 'drm', 'i915')
> +def print_workarounds(project_root, driver_dir, project):
>   olddir = os.getcwd()
> - os.chdir(kernel_path)
> + os.chdir(project_root)
>   work_arounds, err = execute(['git', 'grep', '-n',
>'-e', 'W[aA][A-Z0-9][a-zA-Z0-9_]\+',
> -  i915_dir])
> +  driver_dir])
>   os.chdir(olddir)
>   if err:
>   print(err)
>   sys.exit(1)
>  
>   parse(work_arounds)
> + print "\nList of workarounds found in %s:" % project
>   for wa in sorted(workarounds.keys()):
>   if not options.platform:
>   print("%s: %s" % (wa, ', '.join(workarounds[wa])))
>   elif options.platform in workarounds[wa]:
>   print(wa)
> +
> +
> +if __name__ == '__main__':
> + (options, args) = parse_options(sys.argv)
> + verbose = options.verbose
> + kernel_path = None
> +
> + if not len(args) and options.kernel_path == None and options.mesa_path 
> == None:
> + sys.stderr.write("error: A path to either a kernel tree or Mesa 
> is required\n")
> + sys.exit(1)
> +
> + if len(args):
> + kernel_path = args[0]
> + elif options.kernel_path != None:
> + kernel_path = options.kernel_path
> +
> + if kernel_path != None:
> + # --- list Kernel workarounds if path is provided ---
> + kconfig = os.path.join(kernel_path, 'Kconfig')
> + if not os.path.isfile(kconfig):
> + sys.stderr.write("error: %s does not point to a kernel 
> tree \n"
> + % kernel_path)
> + sys.exit(1)
> +
> + i915_dir = os.path.join('drivers', 'gpu', 'drm', 'i915')
> + print_workarounds(kernel_path, i915_dir, "kernel")
> +
> + # --- list mesa workarounds if path is provided ---
> + if options.mesa_path != None:
> + # reset workarounds array
> +