[Intel-gfx] [PATCH] drm/i915/gvt/kvmgt: remove useless code

[Jike Song]

As noticed by Dan, there is useless (and incorrect) code in kvmgt trying
to kfree(NULL), though almost harmless. It was a copy-paste mistake and
should be removed.

Reported-by: Dan Carpenter 
Signed-off-by: Jike Song 
---
 drivers/gpu/drm/i915/gvt/kvmgt.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index f0a993a..68737dc 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -965,13 +965,7 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, 
unsigned int cmd,
sparse->areas[0].offset =
PAGE_ALIGN(vgpu_aperture_offset(vgpu));
sparse->areas[0].size = vgpu_aperture_sz(vgpu);
-   if (!caps.buf) {
-   kfree(caps.buf);
-   caps.buf = NULL;
-   caps.size = 0;
-   }
break;
-
case VFIO_PCI_BAR3_REGION_INDEX ... VFIO_PCI_BAR5_REGION_INDEX:
info.offset = VFIO_PCI_INDEX_TO_OFFSET(info.index);
info.size = 0;
@@ -979,7 +973,6 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, 
unsigned int cmd,
info.flags = 0;
gvt_dbg_core("get region info bar:%d\n", info.index);
break;
-
case VFIO_PCI_ROM_REGION_INDEX:
case VFIO_PCI_VGA_REGION_INDEX:
gvt_dbg_core("get region info index:%d\n", info.index);
-- 
2.4.4.488.gdf97e5d

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

Re: [Intel-gfx] [PATCH] drm/i915: Remove early pre-production RPS workarounds for BXT

[Joonas Lahtinen]

On ke, 2017-01-25 at 17:26 +, Chris Wilson wrote:
> Remove WaGsvDisableTurbo and WaRsUseTimeoutMode as these were only for
> pre-production Broxton devices, and this code is now defunct.
> 
> Signed-off-by: Chris Wilson 
> Cc: Mika Kuoppala 
> Cc: Joonas Lahtinen 

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [bug report] drm/i915/gvt/kvmgt: add vfio/mdev support to KVMGT

[Jike Song]

On 01/26/2017 02:50 PM, Dan Carpenter wrote:
> Hello Jike Song,
> 
> The patch 659643f7d814: "drm/i915/gvt/kvmgt: add vfio/mdev support to
> KVMGT" from Dec 8, 2016, leads to the following static checker
> warning:
> 
>   drivers/gpu/drm/i915/gvt/kvmgt.c:969 intel_vgpu_ioctl()
>   warn: calling kfree() when 'caps.buf' is always NULL.
> 
> drivers/gpu/drm/i915/gvt/kvmgt.c
>909  } else if (cmd == VFIO_DEVICE_GET_REGION_INFO) {
>910  struct vfio_region_info info;
>911  struct vfio_info_cap caps = { .buf = NULL, .size = 0 
> };
>  ^
> Set here.
> 
>912  int i, ret;
>913  struct vfio_region_info_cap_sparse_mmap *sparse = 
> NULL;
>914  size_t size;
>915  int nr_areas = 1;
>916  int cap_type_id;
>917  
>918  minsz = offsetofend(struct vfio_region_info, offset);
>919  
>920  if (copy_from_user(, (void __user *)arg, minsz))
>921  return -EFAULT;
>922  
>923  if (info.argsz < minsz)
>924  return -EINVAL;
>925  
>926  switch (info.index) {
>927  case VFIO_PCI_CONFIG_REGION_INDEX:
>928  info.offset = 
> VFIO_PCI_INDEX_TO_OFFSET(info.index);
>929  info.size = INTEL_GVT_MAX_CFG_SPACE_SZ;
>930  info.flags = VFIO_REGION_INFO_FLAG_READ |
>931   VFIO_REGION_INFO_FLAG_WRITE;
>932  break;
>933  case VFIO_PCI_BAR0_REGION_INDEX:
>934  info.offset = 
> VFIO_PCI_INDEX_TO_OFFSET(info.index);
>935  info.size = 
> vgpu->cfg_space.bar[info.index].size;
>936  if (!info.size) {
>937  info.flags = 0;
>938  break;
>939  }
>940  
>941  info.flags = VFIO_REGION_INFO_FLAG_READ |
>942   VFIO_REGION_INFO_FLAG_WRITE;
>943  break;
>944  case VFIO_PCI_BAR1_REGION_INDEX:
>945  info.offset = 
> VFIO_PCI_INDEX_TO_OFFSET(info.index);
>946  info.size = 0;
>947  info.flags = 0;
>948  break;
>949  case VFIO_PCI_BAR2_REGION_INDEX:
>950  info.offset = 
> VFIO_PCI_INDEX_TO_OFFSET(info.index);
>951  info.flags = VFIO_REGION_INFO_FLAG_CAPS |
>952  VFIO_REGION_INFO_FLAG_MMAP |
>953  VFIO_REGION_INFO_FLAG_READ |
>954  VFIO_REGION_INFO_FLAG_WRITE;
>955  info.size = gvt_aperture_sz(vgpu->gvt);
>956  
>957  size = sizeof(*sparse) +
>958  (nr_areas * 
> sizeof(*sparse->areas));
>959  sparse = kzalloc(size, GFP_KERNEL);
>960  if (!sparse)
>961  return -ENOMEM;
>962  
>963  sparse->nr_areas = nr_areas;
>964  cap_type_id = 
> VFIO_REGION_INFO_CAP_SPARSE_MMAP;
>965  sparse->areas[0].offset =
>966  
> PAGE_ALIGN(vgpu_aperture_offset(vgpu));
>967  sparse->areas[0].size = 
> vgpu_aperture_sz(vgpu);
>968  if (!caps.buf) {
>  
> It's always NULL.
> 
>969  kfree(caps.buf);
> 
> Freeing NULL is pointless.
> 
>970  caps.buf = NULL;
>971  caps.size = 0;
> 
> These were already zeroed out at the start of the function.  What was
> intended here?  Probably you could just delete these lines.
> 

Hi Dan,

Yes you are right, these are useless, probably a mistake in the development.

Curious what static checker do you use? I actually checked it with sparse but
this was not reported.

I'll submit a patch to remove that, thanks!


--
Thanks,
Jike

>972  }
>973  break;
>974  
>975  case VFIO_PCI_BAR3_REGION_INDEX ... 
> VFIO_PCI_BAR5_REGION_INDEX:
> 
> regards,
> dan carpenter
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org

[Intel-gfx] [bug report] drm/i915/gvt/kvmgt: add vfio/mdev support to KVMGT

[Dan Carpenter]

Hello Jike Song,

The patch 659643f7d814: "drm/i915/gvt/kvmgt: add vfio/mdev support to
KVMGT" from Dec 8, 2016, leads to the following static checker
warning:

drivers/gpu/drm/i915/gvt/kvmgt.c:969 intel_vgpu_ioctl()
warn: calling kfree() when 'caps.buf' is always NULL.

drivers/gpu/drm/i915/gvt/kvmgt.c
   909  } else if (cmd == VFIO_DEVICE_GET_REGION_INFO) {
   910  struct vfio_region_info info;
   911  struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
 ^
Set here.

   912  int i, ret;
   913  struct vfio_region_info_cap_sparse_mmap *sparse = NULL;
   914  size_t size;
   915  int nr_areas = 1;
   916  int cap_type_id;
   917  
   918  minsz = offsetofend(struct vfio_region_info, offset);
   919  
   920  if (copy_from_user(, (void __user *)arg, minsz))
   921  return -EFAULT;
   922  
   923  if (info.argsz < minsz)
   924  return -EINVAL;
   925  
   926  switch (info.index) {
   927  case VFIO_PCI_CONFIG_REGION_INDEX:
   928  info.offset = 
VFIO_PCI_INDEX_TO_OFFSET(info.index);
   929  info.size = INTEL_GVT_MAX_CFG_SPACE_SZ;
   930  info.flags = VFIO_REGION_INFO_FLAG_READ |
   931   VFIO_REGION_INFO_FLAG_WRITE;
   932  break;
   933  case VFIO_PCI_BAR0_REGION_INDEX:
   934  info.offset = 
VFIO_PCI_INDEX_TO_OFFSET(info.index);
   935  info.size = 
vgpu->cfg_space.bar[info.index].size;
   936  if (!info.size) {
   937  info.flags = 0;
   938  break;
   939  }
   940  
   941  info.flags = VFIO_REGION_INFO_FLAG_READ |
   942   VFIO_REGION_INFO_FLAG_WRITE;
   943  break;
   944  case VFIO_PCI_BAR1_REGION_INDEX:
   945  info.offset = 
VFIO_PCI_INDEX_TO_OFFSET(info.index);
   946  info.size = 0;
   947  info.flags = 0;
   948  break;
   949  case VFIO_PCI_BAR2_REGION_INDEX:
   950  info.offset = 
VFIO_PCI_INDEX_TO_OFFSET(info.index);
   951  info.flags = VFIO_REGION_INFO_FLAG_CAPS |
   952  VFIO_REGION_INFO_FLAG_MMAP |
   953  VFIO_REGION_INFO_FLAG_READ |
   954  VFIO_REGION_INFO_FLAG_WRITE;
   955  info.size = gvt_aperture_sz(vgpu->gvt);
   956  
   957  size = sizeof(*sparse) +
   958  (nr_areas * 
sizeof(*sparse->areas));
   959  sparse = kzalloc(size, GFP_KERNEL);
   960  if (!sparse)
   961  return -ENOMEM;
   962  
   963  sparse->nr_areas = nr_areas;
   964  cap_type_id = VFIO_REGION_INFO_CAP_SPARSE_MMAP;
   965  sparse->areas[0].offset =
   966  
PAGE_ALIGN(vgpu_aperture_offset(vgpu));
   967  sparse->areas[0].size = vgpu_aperture_sz(vgpu);
   968  if (!caps.buf) {
 
It's always NULL.

   969  kfree(caps.buf);

Freeing NULL is pointless.

   970  caps.buf = NULL;
   971  caps.size = 0;

These were already zeroed out at the start of the function.  What was
intended here?  Probably you could just delete these lines.

   972  }
   973  break;
   974  
   975  case VFIO_PCI_BAR3_REGION_INDEX ... 
VFIO_PCI_BAR5_REGION_INDEX:

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

[Intel-gfx] [PATCH v3 2/2] drm/i915: clean up unused vgpu_read/write

[Weinan Li]

Having converted the force_wake_get/_put routines for a vGPU to be no-op,
we can use the common mmio accessors and remove our specialised routines
that simply skipped the calls to control force_wake.

Signed-off-by: Weinan Li 
Reviewed-by: Chris Wilson 
---
 drivers/gpu/drm/i915/intel_uncore.c | 58 -
 1 file changed, 58 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
b/drivers/gpu/drm/i915/intel_uncore.c
index 9fad4de..e9046fa 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1052,34 +1052,6 @@ static inline void __force_wake_auto(struct 
drm_i915_private *dev_priv,
 #undef GEN6_READ_FOOTER
 #undef GEN6_READ_HEADER
 
-#define VGPU_READ_HEADER(x) \
-   unsigned long irqflags; \
-   u##x val = 0; \
-   assert_rpm_device_not_suspended(dev_priv); \
-   spin_lock_irqsave(_priv->uncore.lock, irqflags)
-
-#define VGPU_READ_FOOTER \
-   spin_unlock_irqrestore(_priv->uncore.lock, irqflags); \
-   trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \
-   return val
-
-#define __vgpu_read(x) \
-static u##x \
-vgpu_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
-   VGPU_READ_HEADER(x); \
-   val = __raw_i915_read##x(dev_priv, reg); \
-   VGPU_READ_FOOTER; \
-}
-
-__vgpu_read(8)
-__vgpu_read(16)
-__vgpu_read(32)
-__vgpu_read(64)
-
-#undef __vgpu_read
-#undef VGPU_READ_FOOTER
-#undef VGPU_READ_HEADER
-
 #define GEN2_WRITE_HEADER \
trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
assert_rpm_wakelock_held(dev_priv); \
@@ -1202,31 +1174,6 @@ static inline void __force_wake_auto(struct 
drm_i915_private *dev_priv,
 #undef GEN6_WRITE_FOOTER
 #undef GEN6_WRITE_HEADER
 
-#define VGPU_WRITE_HEADER \
-   unsigned long irqflags; \
-   trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
-   assert_rpm_device_not_suspended(dev_priv); \
-   spin_lock_irqsave(_priv->uncore.lock, irqflags)
-
-#define VGPU_WRITE_FOOTER \
-   spin_unlock_irqrestore(_priv->uncore.lock, irqflags)
-
-#define __vgpu_write(x) \
-static void vgpu_write##x(struct drm_i915_private *dev_priv, \
- i915_reg_t reg, u##x val, bool trace) { \
-   VGPU_WRITE_HEADER; \
-   __raw_i915_write##x(dev_priv, reg, val); \
-   VGPU_WRITE_FOOTER; \
-}
-
-__vgpu_write(8)
-__vgpu_write(16)
-__vgpu_write(32)
-
-#undef __vgpu_write
-#undef VGPU_WRITE_FOOTER
-#undef VGPU_WRITE_HEADER
-
 #define ASSIGN_WRITE_MMIO_VFUNCS(x) \
 do { \
dev_priv->uncore.funcs.mmio_writeb = x##_write8; \
@@ -1462,11 +1409,6 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
if (INTEL_GEN(dev_priv) >= 8)
intel_shadow_table_check();
 
-   if (intel_vgpu_active(dev_priv)) {
-   ASSIGN_WRITE_MMIO_VFUNCS(vgpu);
-   ASSIGN_READ_MMIO_VFUNCS(vgpu);
-   }
-
i915_check_and_clear_faults(dev_priv);
 }
 #undef ASSIGN_WRITE_MMIO_VFUNCS
-- 
1.9.1

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

[Intel-gfx] [PATCH v3 1/2] drm/i915: noop forcewake get/put when vgpu activated

[Weinan Li]

For a virtualized GPU, the host maintains the forcewake state on the real
device. As we don't control forcewake ourselves, we can simply set
force_wake_get() and force_wake_put() to be no-ops. By setting the vfuncs,
we adjust both the manual control of forcewake and around the mmio
accessors (making our vgpu specific mmio routines redundant and to be
removed in the next patch).

Signed-off-by: Weinan Li 
Reviewed-by: Chris Wilson 
---
 drivers/gpu/drm/i915/intel_uncore.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
b/drivers/gpu/drm/i915/intel_uncore.c
index abe0888..9fad4de 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -133,6 +133,13 @@
 }
 
 static void
+vgpu_fw_domains_nop(struct drm_i915_private *dev_priv,
+   enum forcewake_domains fw_domains)
+{
+   /* Guest driver doesn't need to takes care forcewake. */
+}
+
+static void
 fw_domains_posting_read(struct drm_i915_private *dev_priv)
 {
struct intel_uncore_forcewake_domain *d;
@@ -1374,6 +1381,12 @@ static void intel_uncore_fw_domains_init(struct 
drm_i915_private *dev_priv)
fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
   FORCEWAKE, FORCEWAKE_ACK);
}
+   if (intel_vgpu_active(dev_priv)) {
+   dev_priv->uncore.funcs.force_wake_get =
+   vgpu_fw_domains_nop;
+   dev_priv->uncore.funcs.force_wake_put =
+   vgpu_fw_domains_nop;
+   }
 
/* All future platforms are expected to require complex power gating */
WARN_ON(dev_priv->uncore.fw_domains == 0);
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH 1/2] drm/i915: ignore posting read when using vgpu

[Li, Weinan Z]

I am not sure if native driver still need POSTING_READ action. But for guest we 
can remove it directly.
Search the code, there are still upon 270 hints use POSTING_READ. 
For guest OS, one frequent POSTING_READ_FW action is in irq handler, it will 
impact the performance if there is heavy interrupts(like media reference usage.)

Thanks.
Best Regards.
Weinan, LI


> -Original Message-
> From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> Sent: Wednesday, January 25, 2017 3:13 PM
> To: Li, Weinan Z 
> Cc: intel-gfx@lists.freedesktop.org; intel-gvt-...@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915: ignore posting read when
> using vgpu
> 
> On Wed, Jan 25, 2017 at 02:04:55PM +0800, Weinan Li wrote:
> > No need to do posting read when vgpu actived.
> 
> No. This is bloat for no gain. Almost all of those posting reads are 
> superfluous
> anyway.
> -Chris
> 
> --
> Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] MAINTAINERS: update new mail list for intel gvt driver

[Zhenyu Wang]

On 2017.01.25 12:49:48 +0200, Jani Nikula wrote:
> Side note, in the admin interface, please allow anyone subscribed to
> intel-gfx or dri-devel post to intel-gvt-dev without moderation. It's
> under Privacy options... | Sender filters | List of non-member addresses
> whose postings should be automatically accepted. Add @intel-gfx and
> @dri-devel there, one per line. (I whitelisted intel-gvt-dev members for
> intel-gfx).
> 

Changed that way. Thanks.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


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

Re: [Intel-gfx] [PATCH v2] drm/dp/mst: fix kernel oops when turning off secondary monitor

[Nathan Ciobanu]

I tested this patch in dinq on a KBL system and it fixed the bug. The 
system doesn't crash on disconnecting or powering off the second monitor 
in the DP-MST chain. I also replied to the Bugzilla issue.


Tested-by: Nathan D Ciobanu 

On 12/05/2016 01:49 PM, Pierre-Louis Bossart wrote:

100% reproducible issue found on SKL SkullCanyon NUC with two external
DP daisy-chained monitors in DP/MST mode. When turning off or changing
the input of the second monitor the machine stops with a kernel
oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.

This issue is traced to an inconsistent control flow in
drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at
the same time as 'req_payload.num_slots' is set to zero, but the pointer
is dereferenced even when req_payload.num_slot is zero.

The problematic dereference was introduced in commit dfda0df34
("drm/mst: rework payload table allocation to conform better")
and may impact all versions since v3.18

The fix suggested by Chris Wilson removes the kernel oops and was found to
work well after 10mn of monkey-testing with the second monitor power and
input buttons

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
Cc: Dave Airlie 
Signed-off-by: Pierre-Louis Bossart 
---
  drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index aa64448..f59771d 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct 
drm_dp_mst_topology_mgr *mgr)
mgr->payloads[i].vcpi = req_payload.vcpi;
} else if (mgr->payloads[i].num_slots) {
mgr->payloads[i].num_slots = 0;
-   drm_dp_destroy_payload_step1(mgr, port, 
port->vcpi.vcpi, >payloads[i]);
+   drm_dp_destroy_payload_step1(mgr, port, 
mgr->payloads[i].vcpi, >payloads[i]);
req_payload.payload_state = 
mgr->payloads[i].payload_state;
mgr->payloads[i].start_slot = 0;
}


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

Re: [Intel-gfx] [PATCH 07/15] drm/gma500: Nuke device_is_agp callback

[Patrik Jakobsson]

On Wed, Jan 25, 2017 at 7:26 AM, Daniel Vetter  wrote:
> Returning 0 for an on-chip gpu doesn't change anything at all.
>
> Cc: Patrik Jakobsson 
> Signed-off-by: Daniel Vetter 

Acked-by: Patrik Jakobsson 

> ---
>  drivers/gpu/drm/gma500/psb_drv.c | 6 --
>  1 file changed, 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/psb_drv.c 
> b/drivers/gpu/drm/gma500/psb_drv.c
> index 0dc7ba2fdc22..5ee93ff55608 100644
> --- a/drivers/gpu/drm/gma500/psb_drv.c
> +++ b/drivers/gpu/drm/gma500/psb_drv.c
> @@ -406,11 +406,6 @@ static int psb_driver_load(struct drm_device *dev, 
> unsigned long flags)
> return ret;
>  }
>
> -static int psb_driver_device_is_agp(struct drm_device *dev)
> -{
> -   return 0;
> -}
> -
>  static inline void get_brightness(struct backlight_device *bd)
>  {
>  #ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
> @@ -487,7 +482,6 @@ static struct drm_driver driver = {
> .set_busid = drm_pci_set_busid,
>
> .num_ioctls = ARRAY_SIZE(psb_ioctls),
> -   .device_is_agp = psb_driver_device_is_agp,
> .irq_preinstall = psb_irq_preinstall,
> .irq_postinstall = psb_irq_postinstall,
> .irq_uninstall = psb_irq_uninstall,
> --
> 2.11.0
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/dp/mst: fix kernel oops when turning off secondary monitor

[Nathan Ciobanu]

I tested this on a KBL using 01-25-2017 dinq and it fixed the bug.


Tested-by: Nathan D Ciobanu 


On 12/05/2016 01:49 PM, Pierre-Louis Bossart wrote:

100% reproducible issue found on SKL SkullCanyon NUC with two external
DP daisy-chained monitors in DP/MST mode. When turning off or changing
the input of the second monitor the machine stops with a kernel
oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.

This issue is traced to an inconsistent control flow in
drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at
the same time as 'req_payload.num_slots' is set to zero, but the pointer
is dereferenced even when req_payload.num_slot is zero.

The problematic dereference was introduced in commit dfda0df34
("drm/mst: rework payload table allocation to conform better")
and may impact all versions since v3.18

The fix suggested by Chris Wilson removes the kernel oops and was found to
work well after 10mn of monkey-testing with the second monitor power and
input buttons

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
Cc: Dave Airlie 
Signed-off-by: Pierre-Louis Bossart 
---
  drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index aa64448..f59771d 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct 
drm_dp_mst_topology_mgr *mgr)
mgr->payloads[i].vcpi = req_payload.vcpi;
} else if (mgr->payloads[i].num_slots) {
mgr->payloads[i].num_slots = 0;
-   drm_dp_destroy_payload_step1(mgr, port, 
port->vcpi.vcpi, >payloads[i]);
+   drm_dp_destroy_payload_step1(mgr, port, 
mgr->payloads[i].vcpi, >payloads[i]);
req_payload.payload_state = 
mgr->payloads[i].payload_state;
mgr->payloads[i].start_slot = 0;
}


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

Re: [Intel-gfx] [PATCH v5 0/5] Add support for Legacy HDMI audio drivers

[Pierre-Louis Bossart]

On 1/25/17 3:21 PM, Takashi Iwai wrote:

On Tue, 24 Jan 2017 23:57:48 +0100,
Jerome Anand wrote:


This patch series has only been tested on hardware with
a single HDMI connector/pipe and additional work may be needed for newer
machines with 2 connectors


BTW, I have such a machine, CHV with two DP outputs.
If you have a test patch (not necessarily in a mergeable form), I'll
be glad to test.  This will make me easier to work on further
cleanups.


I also have a CHT machine with 1 DP and 1 HDMI connector. I'll start 
working on DP once I manage to find some time.


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

Re: [Intel-gfx] [PATCH v2 4/9] drm: Add driver private objects to atomic state

[Harry Wentland]

On 2017-01-25 12:59 AM, Daniel Vetter wrote:
> On Tue, Jan 24, 2017 at 03:49:32PM -0800, Dhinakaran Pandiyan wrote:
>> It is necessary to track states for objects other than connector, crtc
>> and plane for atomic modesets. But adding objects like DP MST link
>> bandwidth to drm_atomic_state would mean that a non-core object will be
>> modified by the core helper functions for swapping and clearing
>> it's state. So, lets add void * objects and helper functions that operate
>> on void * types to keep these objects and states private to the core.
>> Drivers can then implement specific functions to swap and clear states.
>> The other advantage having just void * for these objects in
>> drm_atomic_state is that objects of different types can be managed in the
>> same state array.
>>
>> Suggested-by: Daniel Vetter 
>> Signed-off-by: Dhinakaran Pandiyan 
> 
> I think an overview DOC: section to explain how to subclass atomic state
> and how to do private state objects would be great. But I can do that once
> your patch has landed, since it'd be much more about the overall design of
> atomic (and I promised to do that anyway).
> 
> Looks pretty good, a few nits below still. I'll also ask amd folks to
> check this out, and I think Ville could use it for his cdclk stuff.

Thanks for pinging me again about this. This should help with attaching
our context to atomic_state in a clean fashion. I hope to show some
patches of it soon after I rebase on top of drm-next + these patches.

> 
>> ---
>>  drivers/gpu/drm/drm_atomic.c| 55 
>> +
>>  drivers/gpu/drm/drm_atomic_helper.c |  6 
>>  include/drm/drm_atomic.h| 30 
>>  3 files changed, 91 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index 723392f..f3a71cc 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -57,6 +57,7 @@ void drm_atomic_state_default_release(struct 
>> drm_atomic_state *state)
>>  kfree(state->connectors);
>>  kfree(state->crtcs);
>>  kfree(state->planes);
>> +kfree(state->private_objs);
>>  }
>>  EXPORT_SYMBOL(drm_atomic_state_default_release);
>>  
>> @@ -184,6 +185,20 @@ void drm_atomic_state_default_clear(struct 
>> drm_atomic_state *state)
>>  state->planes[i].ptr = NULL;
>>  state->planes[i].state = NULL;
>>  }
>> +
>> +for (i = 0; i < state->num_private_objs; i++) {
>> +void *priv_obj = state->private_objs[i].obj;
>> +void *obj_state = state->private_objs[i].obj_state;
>> +
>> +if (!priv_obj)
>> +continue;
>> +
>> +state->private_objs[i].funcs->destroy_state(obj_state);
>> +state->private_objs[i].obj = NULL;
>> +state->private_objs[i].obj_state = NULL;
>> +state->private_objs[i].funcs = NULL;
>> +}
>> +
>>  }
>>  EXPORT_SYMBOL(drm_atomic_state_default_clear);
>>  
>> @@ -976,6 +991,46 @@ static void drm_atomic_plane_print_state(struct 
>> drm_printer *p,
>>  plane->funcs->atomic_print_state(p, state);
>>  }
>>  
>> +
> 
> Needs kerneldoc here.
> 
>> +void *
>> +drm_atomic_get_priv_obj_state(struct drm_atomic_state *state, void *obj,
> 
> ocd bikeshed: priv_obj vs private_obj inconsistency here, please stick to
> one (I don't care which one).
> 
>> +  const struct drm_private_state_funcs *funcs)
>> +{
>> +int index, num_objs, i;
>> +size_t size;
>> +struct __drm_private_objs_state *arr;
>> +
>> +for (i = 0; i < state->num_private_objs; i++)
>> +if (obj == state->private_objs[i].obj &&
>> +state->private_objs[i].obj_state)
>> +return state->private_objs[i].obj_state;
>> +
>> +num_objs = state->num_private_objs + 1;
>> +size = sizeof(*state->private_objs) * num_objs;
>> +arr = krealloc(state->private_objs, size, GFP_KERNEL);
>> +if (!arr)
>> +return ERR_PTR(-ENOMEM);
>> +
>> +state->private_objs = arr;
>> +index = state->num_private_objs;
>> +memset(>private_objs[index], 0, sizeof(*state->private_objs));
>> +
>> +state->private_objs[index].obj_state = funcs->duplicate_state(state, 
>> obj);
>> +if (!state->private_objs[index].obj_state)
>> +return ERR_PTR(-ENOMEM);
> 
> I wondered whether we need to allow other error codes than ENOMEM, e.g.
> if duplicate_state needs to acquire a ww_mutex. But we can always acquire
> the necessary locks outside of drm_atomic_get_priv_obj_state in some
> helper/driver wrapper. So no big deal, but worth explaining that
> drm_atomic_get_priv_obj_state won't acquire necessarily locsk (since it
> doesn't know about them) in the kernel-doc.
> 
>> +
>> +state->private_objs[index].obj = obj;
>> +state->private_objs[index].funcs = funcs;
>> +state->num_private_objs = num_objs;
>> +

Re: [Intel-gfx] [PATCH v5 0/5] Add support for Legacy HDMI audio drivers

[Takashi Iwai]

On Tue, 24 Jan 2017 23:57:48 +0100,
Jerome Anand wrote:
> 
> This patch series has only been tested on hardware with 
> a single HDMI connector/pipe and additional work may be needed for newer 
> machines with 2 connectors

BTW, I have such a machine, CHV with two DP outputs.
If you have a test patch (not necessarily in a mergeable form), I'll
be glad to test.  This will make me easier to work on further
cleanups.


thanks,

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

Re: [Intel-gfx] [PATCH] ALSA: x86: hdmi: fix returnvar.cocci warnings

[Takashi Iwai]

On Tue, 24 Jan 2017 17:07:48 +0100,
Julia Lawall wrote:
> 
> Remove unneeded variable used to store return value.
> 
> Generated by: scripts/coccinelle/misc/returnvar.cocci
> 
> CC: Jerome Anand 
> Signed-off-by: Julia Lawall 
> Signed-off-by: Fengguang Wu 
> ---
> 
> In-Reply-To: <20170124225753.9045-5-jerome.an...@intel.com>
> url:
> https://github.com/0day-ci/linux/commits/Jerome-Anand/Add-support-for-Legacy
> -HDMI-audio-drivers/20170124-213547
> 
> This is probably not the right patch.  It si just what the rule generates.
> Instead, I guess that retval should be updated in some way?

Yeah, it looks a bit suspicious, but it can be fixed later.
So I applied your cleanup now.


thanks,

Takashi

> 
>  intel_hdmi_audio_if.c |   21 +
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> --- a/sound/x86/intel_hdmi_audio_if.c
> +++ b/sound/x86/intel_hdmi_audio_if.c
> @@ -239,7 +239,6 @@ static inline int had_chk_intrmiss(struc
> 
>  int had_process_buffer_done(struct snd_intelhad *intelhaddata)
>  {
> - int retval = 0;
>   u32 len = 1;
>   enum intel_had_aud_buf_type buf_id;
>   enum intel_had_aud_buf_type buff_done;
> @@ -258,7 +257,7 @@ int had_process_buffer_done(struct snd_i
>   if (intelhaddata->drv_status == HAD_DRV_DISCONNECTED) {
>   spin_unlock_irqrestore(>had_spinlock, flag_irqs);
>   pr_err("%s:Device already disconnected\n", __func__);
> - return retval;
> + return 0;
>   }
>   buf_id = intelhaddata->curr_buf;
>   intelhaddata->buff_done = buf_id;
> @@ -280,7 +279,7 @@ int had_process_buffer_done(struct snd_i
>   if (!intr_count || (intr_count > 3)) {
>   pr_err("HAD SW state in non-recoverable!!! mode\n");
>   pr_err("Already played stale data\n");
> - return retval;
> + return 0;
>   }
>   buf_id += (intr_count - 1);
>   buf_id = buf_id % 4;
> @@ -298,7 +297,7 @@ int had_process_buffer_done(struct snd_i
> 
>   if (had_get_hwstate(intelhaddata)) {
>   pr_err("HDMI cable plugged-out\n");
> - return retval;
> + return 0;
>   }
> 
>   /*Reprogram the registers with addr and length*/
> @@ -322,12 +321,11 @@ int had_process_buffer_done(struct snd_i
>   stream->period_elapsed(stream->had_substream);
>   }
> 
> - return retval;
> + return 0;
>  }
> 
>  int had_process_buffer_underrun(struct snd_intelhad *intelhaddata)
>  {
> - int retval = 0;
>   enum intel_had_aud_buf_type buf_id;
>   struct pcm_stream_info *stream;
>   struct had_pvt_data *had_stream;
> @@ -355,7 +353,7 @@ int had_process_buffer_underrun(struct s
> 
>   if (drv_status == HAD_DRV_DISCONNECTED) {
>   pr_err("%s:Device already disconnected\n", __func__);
> - return retval;
> + return 0;
>   }
> 
>   if (stream_type == HAD_RUNNING_STREAM) {
> @@ -364,12 +362,11 @@ int had_process_buffer_underrun(struct s
>   stream->period_elapsed(stream->had_substream);
>   }
> 
> - return retval;
> + return 0;
>  }
> 
>  int had_process_hot_plug(struct snd_intelhad *intelhaddata)
>  {
> - int retval = 0;
>   enum intel_had_aud_buf_type buf_id;
>   struct snd_pcm_substream *substream;
>   struct had_pvt_data *had_stream;
> @@ -384,7 +381,7 @@ int had_process_hot_plug(struct snd_inte
>   if (intelhaddata->drv_status == HAD_DRV_CONNECTED) {
>   pr_debug("Device already connected\n");
>   spin_unlock_irqrestore(>had_spinlock, flag_irqs);
> - return retval;
> + return 0;
>   }
>   buf_id = intelhaddata->curr_buf;
>   intelhaddata->buff_done = buf_id;
> @@ -422,12 +419,12 @@ int had_process_hot_plug(struct snd_inte
> 
>   had_build_channel_allocation_map(intelhaddata);
> 
> - return retval;
> + return 0;
> 
>  err:
>   pm_runtime_disable(intelhaddata->dev);
>   intelhaddata->dev = NULL;
> - return retval;
> + return 0;
>  }
> 
>  int had_process_hot_unplug(struct snd_intelhad *intelhaddata)
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 7/9] drm: Connector helper function to release atomic state

[Pandiyan, Dhinakaran]

On Wed, 2017-01-25 at 07:18 +0100, Daniel Vetter wrote:
> On Tue, Jan 24, 2017 at 03:49:35PM -0800, Dhinakaran Pandiyan wrote:
> > Having a ->atomic_release callback is useful to release shared resources
> > that get allocated in compute_config().
> > 
> > Suggested-by: Daniel Vetter 
> > Signed-off-by: Dhinakaran Pandiyan 
> > ---
> >  include/drm/drm_modeset_helper_vtables.h | 15 +++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/include/drm/drm_modeset_helper_vtables.h 
> > b/include/drm/drm_modeset_helper_vtables.h
> > index 46f5b34..e41b18a 100644
> > --- a/include/drm/drm_modeset_helper_vtables.h
> > +++ b/include/drm/drm_modeset_helper_vtables.h
> > @@ -831,6 +831,21 @@ struct drm_connector_helper_funcs {
> >  */
> > struct drm_encoder *(*atomic_best_encoder)(struct drm_connector 
> > *connector,
> >struct drm_connector_state 
> > *connector_state);
> > +
> > +   /**
> > +* @atomic_release:
> > +*
> > +* This function is used to release shared resources that were
> > +* previously acquired. For example, resources acquired in
> > +* encoder->compute_config() can be released by calling this function
> 
> @compute_config is the right way to do references within the same struct.
> 
> > +* from mode_fixup()
> 
> Same here.
> 
> Patch split up is a bit strange, hence why my review of the design is in
> later patches.
> 
> Thanks, Daniel
> 


I'll squash them appropriately, thanks for the review.

-DK
> > +*
> > +* NOTE:
> > +*
> > +* This function is called in the check phase of an atomic update.
> > +*/
> > +   void (*atomic_release)(struct drm_connector *connector,
> > +  struct drm_connector_state *connector_state);
> >  };
> >  
> >  /**
> > -- 
> > 2.7.4
> > 
> 

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

Re: [Intel-gfx] [PATCH v2 9/9] drm/dp: Track MST link bandwidth

[Pandiyan, Dhinakaran]

On Wed, 2017-01-25 at 07:15 +0100, Daniel Vetter wrote:
> On Tue, Jan 24, 2017 at 03:49:37PM -0800, Dhinakaran Pandiyan wrote:
> > Use the added helpers to track MST link bandwidth for atomic modesets.
> > Link bw is acquired in the ->atomic_check() phase when CRTCs are being
> > enabled with drm_atomic_find_vcpi_slots() instead of drm_find_vcpi_slots().
> > Similarly, link bw is released during ->atomic_check() with the connector
> > helper callback ->atomic_release() when CRTCs are disabled.
> > 
> > Signed-off-by: Dhinakaran Pandiyan 
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c |  9 -
> >  drivers/gpu/drm/i915/intel_dp_mst.c | 13 -
> >  2 files changed, 20 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index dd34d21..0d42173 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -385,8 +385,15 @@ mode_fixup(struct drm_atomic_state *state)
> >  
> > WARN_ON(!!conn_state->best_encoder != !!conn_state->crtc);
> >  
> > -   if (!conn_state->crtc || !conn_state->best_encoder)
> > +   if (!conn_state->crtc || !conn_state->best_encoder) {
> > +   const struct drm_connector_helper_funcs *conn_funcs;
> > +
> > +   conn_funcs = connector->helper_private;
> > +   if (conn_funcs->atomic_release)
> > +   conn_funcs->atomic_release(connector,
> > +  conn_state);
> 
> I wonder whether this won't surprise drivers: The way you coded this
> release only gets called when the connector is fully disabled. But it does
> not get called when you atomically switch it from one crtc to the other
> (in which case you also might want to release resources, before allocating
> new ones). I think that case of directly switching stuff is even a bug in
> your current atomic tracking code ...
> 
> We also need to extract the release calls into a separate loop which runs
> _before_ we allocate any resources. Otherwise if you do an atomic commit
> where you disable connector2 and enable connector1 and they can't run both
> at the same time it'll fail, because we'll release the vcpi for connector2
> only after we've tried to get it for connnector1.
> 


Yeah, you are right. I thought of this problem and then forgot about it.
Is there any igt test to test the switching?


-DK
> > continue;
> > +   }
> >  
> > crtc_state = drm_atomic_get_existing_crtc_state(state,
> > 
> > conn_state->crtc);
> 
> Misplaced hunk, this needs to be in the patch that adds the
> ->atomic_release callback.
> 
> > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
> > b/drivers/gpu/drm/i915/intel_dp_mst.c
> > index 2f57a56..ee5fedb 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > @@ -44,6 +44,7 @@ static bool intel_dp_mst_compute_config(struct 
> > intel_encoder *encoder,
> > int lane_count, slots;
> > const struct drm_display_mode *adjusted_mode = 
> > _config->base.adjusted_mode;
> > int mst_pbn;
> > +   struct drm_dp_mst_topology_state *topology_state;
> >  
> > pipe_config->has_pch_encoder = false;
> > bpp = 24;
> > @@ -65,7 +66,17 @@ static bool intel_dp_mst_compute_config(struct 
> > intel_encoder *encoder,
> > mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
> >  
> > pipe_config->pbn = mst_pbn;
> > -   slots = drm_dp_find_vcpi_slots(_dp->mst_mgr, mst_pbn);
> > +   topology_state = drm_atomic_get_mst_topology_state(state,
> > +  _dp->mst_mgr);
> > +   if (topology_state == NULL)
> > +   return false;
> > +
> > +   slots = drm_dp_atomic_find_vcpi_slots(topology_state, connector->port,
> > + mst_pbn);
> 
> Can we merge the get_mst_topology_state and find_vcpi_slots functions
> together? Would be less code in drivers ...
> 
> > +   if (slots < 0) {
> > +   DRM_DEBUG_KMS("not enough link bw for this mode\n");
> > +   return false;
> > +   }
> 
> And then also stuff the error checking in there, and just have a return
> -EINVAL when there's not enough bw.
> 
> I think this should be together with the previous patch, to wire up the
> entire mst state tracking for i915 at the same time. Atm the previous
> patch just wires up dead code, which is a bit backwards.
> -Daniel
> 
> >  
> > intel_link_compute_m_n(bpp, lane_count,
> >adjusted_mode->crtc_clock,
> > -- 
> > 2.7.4
> > 
> 

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

Re: [Intel-gfx] [PATCH v2 8/9] drm/dp: Release DP MST shared link bandwidth

[Pandiyan, Dhinakaran]

On Wed, 2017-01-25 at 07:16 +0100, Daniel Vetter wrote:
> On Tue, Jan 24, 2017 at 03:49:36PM -0800, Dhinakaran Pandiyan wrote:
> > Implement the ->atomic_release() callback to release the shared link
> > bandwidth that was originally acquired during compute_config()
> > 
> > Signed-off-by: Dhinakaran Pandiyan 
> > ---
> >  drivers/gpu/drm/i915/intel_dp_mst.c | 28 
> >  1 file changed, 28 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
> > b/drivers/gpu/drm/i915/intel_dp_mst.c
> > index f51574f..2f57a56 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > @@ -78,6 +78,33 @@ static bool intel_dp_mst_compute_config(struct 
> > intel_encoder *encoder,
> >  
> >  }
> >  
> > +static void intel_dp_mst_atomic_release(struct drm_connector *connector,
> > +   struct drm_connector_state *conn_state)
> > +{
> > +   struct intel_dp_mst_encoder *intel_mst;
> > +   struct drm_dp_mst_topology_mgr *mgr;
> > +   struct drm_dp_mst_topology_state *topology_state;
> > +   struct drm_encoder *encoder;
> > +   struct intel_connector *intel_connector = to_intel_connector(connector);
> > +
> > +   encoder = connector->state->best_encoder;
> > +   if (!encoder || to_intel_encoder(encoder)->type != INTEL_OUTPUT_DP_MST)
> > +   return;
> 
> NULL encoder should imo be caught in core. Type != DP_MST is impossible,
> if you're unsure make it into a BUG_ON for testing.


I don't get why (type != INTEL_OUTPUT_DP_MST) is impossible. Is that
because we have the implementation for atomic_release() only for MST
connectors? But, that is only for now.

-DK
> > +
> > +   intel_mst = enc_to_mst(encoder);
> > +   mgr = _mst->primary->dp.mst_mgr;
> > +
> > +   topology_state = drm_atomic_get_mst_topology_state(conn_state->state,
> > +  mgr);
> > +   if (IS_ERR(topology_state)) {
> > +   DRM_DEBUG_KMS("failed to create MST topology state %ld\n",
> > +   PTR_ERR(topology_state));
> > +   return;
> > +   }
> > +
> > +   drm_dp_atomic_release_vcpi_slots(topology_state, intel_connector->port);
> 
> I think it'd be great to merge this together into 1 helper that both gets
> the state and releases the vcpi, for less code in drivers.

Agreed.

> > +}
> > +
> >  static void intel_mst_disable_dp(struct intel_encoder *encoder,
> >  struct intel_crtc_state *old_crtc_state,
> >  struct drm_connector_state *old_conn_state)
> > @@ -401,6 +428,7 @@ static const struct drm_connector_helper_funcs 
> > intel_dp_mst_connector_helper_fun
> > .mode_valid = intel_dp_mst_mode_valid,
> > .atomic_best_encoder = intel_mst_atomic_best_encoder,
> > .best_encoder = intel_mst_best_encoder,
> > +   .atomic_release = intel_dp_mst_atomic_release,
> >  };
> >  
> >  static void intel_dp_mst_encoder_destroy(struct drm_encoder *encoder)
> > -- 
> > 2.7.4
> > 
> 

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

Re: [Intel-gfx] [PATCH v2 4/9] drm: Add driver private objects to atomic state

[Pandiyan, Dhinakaran]

On Wed, 2017-01-25 at 06:59 +0100, Daniel Vetter wrote:
> On Tue, Jan 24, 2017 at 03:49:32PM -0800, Dhinakaran Pandiyan wrote:
> > It is necessary to track states for objects other than connector, crtc
> > and plane for atomic modesets. But adding objects like DP MST link
> > bandwidth to drm_atomic_state would mean that a non-core object will be
> > modified by the core helper functions for swapping and clearing
> > it's state. So, lets add void * objects and helper functions that operate
> > on void * types to keep these objects and states private to the core.
> > Drivers can then implement specific functions to swap and clear states.
> > The other advantage having just void * for these objects in
> > drm_atomic_state is that objects of different types can be managed in the
> > same state array.
> > 
> > Suggested-by: Daniel Vetter 
> > Signed-off-by: Dhinakaran Pandiyan 
> 
> I think an overview DOC: section to explain how to subclass atomic state
> and how to do private state objects would be great. But I can do that once
> your patch has landed, since it'd be much more about the overall design of
> atomic (and I promised to do that anyway).
> 
> Looks pretty good, a few nits below still. I'll also ask amd folks to
> check this out, and I think Ville could use it for his cdclk stuff.
> 
> > ---
> >  drivers/gpu/drm/drm_atomic.c| 55 
> > +
> >  drivers/gpu/drm/drm_atomic_helper.c |  6 
> >  include/drm/drm_atomic.h| 30 
> >  3 files changed, 91 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 723392f..f3a71cc 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -57,6 +57,7 @@ void drm_atomic_state_default_release(struct 
> > drm_atomic_state *state)
> > kfree(state->connectors);
> > kfree(state->crtcs);
> > kfree(state->planes);
> > +   kfree(state->private_objs);
> >  }
> >  EXPORT_SYMBOL(drm_atomic_state_default_release);
> >  
> > @@ -184,6 +185,20 @@ void drm_atomic_state_default_clear(struct 
> > drm_atomic_state *state)
> > state->planes[i].ptr = NULL;
> > state->planes[i].state = NULL;
> > }
> > +
> > +   for (i = 0; i < state->num_private_objs; i++) {
> > +   void *priv_obj = state->private_objs[i].obj;
> > +   void *obj_state = state->private_objs[i].obj_state;
> > +
> > +   if (!priv_obj)
> > +   continue;
> > +
> > +   state->private_objs[i].funcs->destroy_state(obj_state);
> > +   state->private_objs[i].obj = NULL;
> > +   state->private_objs[i].obj_state = NULL;
> > +   state->private_objs[i].funcs = NULL;
> > +   }
> > +
> >  }
> >  EXPORT_SYMBOL(drm_atomic_state_default_clear);
> >  
> > @@ -976,6 +991,46 @@ static void drm_atomic_plane_print_state(struct 
> > drm_printer *p,
> > plane->funcs->atomic_print_state(p, state);
> >  }
> >  
> > +
> 
> Needs kerneldoc here.
> 
> > +void *
> > +drm_atomic_get_priv_obj_state(struct drm_atomic_state *state, void *obj,
> 
> ocd bikeshed: priv_obj vs private_obj inconsistency here, please stick to
> one (I don't care which one).
> 
> > + const struct drm_private_state_funcs *funcs)
> > +{
> > +   int index, num_objs, i;
> > +   size_t size;
> > +   struct __drm_private_objs_state *arr;
> > +
> > +   for (i = 0; i < state->num_private_objs; i++)
> > +   if (obj == state->private_objs[i].obj &&
> > +   state->private_objs[i].obj_state)
> > +   return state->private_objs[i].obj_state;
> > +
> > +   num_objs = state->num_private_objs + 1;
> > +   size = sizeof(*state->private_objs) * num_objs;
> > +   arr = krealloc(state->private_objs, size, GFP_KERNEL);
> > +   if (!arr)
> > +   return ERR_PTR(-ENOMEM);
> > +
> > +   state->private_objs = arr;
> > +   index = state->num_private_objs;
> > +   memset(>private_objs[index], 0, sizeof(*state->private_objs));
> > +
> > +   state->private_objs[index].obj_state = funcs->duplicate_state(state, 
> > obj);
> > +   if (!state->private_objs[index].obj_state)
> > +   return ERR_PTR(-ENOMEM);
> 
> I wondered whether we need to allow other error codes than ENOMEM, e.g.
> if duplicate_state needs to acquire a ww_mutex. But we can always acquire
> the necessary locks outside of drm_atomic_get_priv_obj_state in some
> helper/driver wrapper. So no big deal, but worth explaining that
> drm_atomic_get_priv_obj_state won't acquire necessarily locsk (since it
> doesn't know about them) in the kernel-doc.
> 

Got it, will include that.

> > +
> > +   state->private_objs[index].obj = obj;
> > +   state->private_objs[index].funcs = funcs;
> > +   state->num_private_objs = num_objs;
> > +
> > +   DRM_DEBUG_ATOMIC("Added new private object state %p to %p\n",
> > +

Re: [Intel-gfx] drm/i915 connector list locking issues..

[Linus Torvalds]

On Wed, Jan 25, 2017 at 12:13 PM, Linus Torvalds
 wrote:
>
> Is nobody else seeing this? This is on my bog-standard intel laptop
> (Dell XPS13 as you can see in the warning).

Actually, it's on my desktop as well. So I'm assuming pretty much any
i915 config shows it. It's been introduced after rc5.

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

Re: [Intel-gfx] [PATCH v3 13/14] drm/i915: Enable userspace to opt-out of implicit fencing

[Chad Versace]

On Mon 14 Nov 2016, Chris Wilson wrote:
> Userspace is faced with a dilemma. The kernel requires implicit fencing
> to manage resource usage (we always must wait for the GPU to finish
> before releasing its PTE) and for third parties. However, userspace may
> wish to avoid this serialisation if it is either using explicit fencing
> between parties and wants more fine-grained access to buffers (e.g. it
> may partition the buffer between uses and track fences on ranges rather
> than the implicit fences tracking the whole object). It follows that
> userspace needs a mechanism to avoid the kernel's serialisation on its
> implicit fences before execbuf execution.
> 
> The next question is whether this is an object, execbuf or context flag.
> Hybrid users (such as using explicit EGL_ANDROID_native_sync fencing on
> shared winsys buffers, but implicit fencing on internal surfaces)
> require a per-object level flag. Given that this flag need to be only
> set once for the lifetime of the object, this reduces the convenience of
> having an execbuf or context level flag (and avoids having multiple
> pieces of uABI controlling the same feature).
> 
> Incorrect use of this flag will result in rendering corruption and GPU
> hangs - but will not result in use-after-free or similar resource
> tracking issues.
> 
> Serious caveat: write ordering is not strictly correct after setting
> this flag on a render target on multiple engines. This affects all
> subsequent GEM operations (execbuf, set-domain, pread) and shared
> dma-buf operations. A fix is possible - but costly (both in terms of
> further ABI changes and runtime overhead).
> 
> Testcase: igt/gem_exec_async
> Signed-off-by: Chris Wilson 
> Reviewed-by: Joonas Lahtinen 
> ---
>  drivers/gpu/drm/i915/i915_drv.c|  1 +
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  3 +++
>  include/uapi/drm/i915_drm.h| 29 -
>  3 files changed, 32 insertions(+), 1 deletion(-)

I'm neutral about this patch. I believe patch 14/14 is useful with or
without this patch, and I want to see patch 14 land regardless of what
happens with this one.

I'm not opposed to this patch. It's just that I don't yet understand
exactly if Mesa's EGL/GL code could effectively use this feature for
Android winsys buffers. The amount of information loss between the
EGL/GL apis and the eventual execbuffer submission may prevent Mesa from
annotating the Android winsys buffers with this.  I'm unsure.  I'm still
thinking about it.

But, if Chris, or anyone, already has plans to use this somehow, perhaps
in the DDX, then don't let my hesitation block the patch.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 2/9] drm/dp: Kill unused MST vcpi slot availability tracking

[Pandiyan, Dhinakaran]

On Wed, 2017-01-25 at 06:43 +0100, Daniel Vetter wrote:
> On Tue, Jan 24, 2017 at 03:49:30PM -0800, Dhinakaran Pandiyan wrote:
> > The avail_slots member in the MST topology manager is never updated to
> > reflect the available vcpi slots. The check is effectively against
> > total_slots. So, let's make that check obvious. Secondly, since the total
> > vcpi time slots is always 63 and does not depend on the link BW, remove
> > total_slots from MST topology manager struct. The third change is to
> > remove total_pbn which is hardcoded to 2560. The total PBN that the
> > topology manager allocates from depends on the link rate and is not a
> > constant. So, fix this by removing the total_pbn member itself. Finally,
> > make debug messages more descriptive.
> 
> Ok, these are 3 different changes, and they need to be split up. Well, at
> least in 2 patches, to get the functional change out of there:
> - First hardcode avail_slots to 63, with the commit message explaining in
>   detail why that's the right thing. You can already remove total_pbn and
>   total_slots in that patch, since they will be unused. The commit message
>   should have a reference to the DP spec to support that "it's always 63"
>   claim.
> - Then garbage-collect ->avail_slots in a 2nd patch.
> 
> If you smash these together it's a lot less obvious that this is not just
> a pure refactoring.
> 
> Thanks, Daniel
> 

Makes sense, will do this in the next revision.

-DK
> > 
> > Signed-off-by: Dhinakaran Pandiyan 
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c | 16 
> >  include/drm/drm_dp_mst_helper.h   | 12 
> >  2 files changed, 8 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 122a1b0..d9edd84 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -2042,10 +2042,6 @@ int drm_dp_mst_topology_mgr_set_mst(struct 
> > drm_dp_mst_topology_mgr *mgr, bool ms
> > goto out_unlock;
> > }
> >  
> > -   mgr->total_pbn = 2560;
> > -   mgr->total_slots = DIV_ROUND_UP(mgr->total_pbn, mgr->pbn_div);
> > -   mgr->avail_slots = mgr->total_slots;
> > -
> > /* add initial branch device at LCT 1 */
> > mstb = drm_dp_add_mst_branch_device(1, NULL);
> > if (mstb == NULL) {
> > @@ -2475,7 +2471,8 @@ int drm_dp_find_vcpi_slots(struct 
> > drm_dp_mst_topology_mgr *mgr,
> >  
> > num_slots = DIV_ROUND_UP(pbn, mgr->pbn_div);
> >  
> > -   if (num_slots > mgr->avail_slots)
> > +   /* max. time slots - one slot for MTP header */
> > +   if (num_slots > 63)
> > return -ENOSPC;
> > return num_slots;
> >  }
> > @@ -2489,7 +2486,8 @@ static int drm_dp_init_vcpi(struct 
> > drm_dp_mst_topology_mgr *mgr,
> >  
> > num_slots = DIV_ROUND_UP(pbn, mgr->pbn_div);
> >  
> > -   if (num_slots > mgr->avail_slots)
> > +   /* max. time slots - one slot for MTP header */
> > +   if (num_slots > 63)
> > return -ENOSPC;
> >  
> > vcpi->pbn = pbn;
> > @@ -2528,10 +2526,12 @@ bool drm_dp_mst_allocate_vcpi(struct 
> > drm_dp_mst_topology_mgr *mgr, struct drm_dp
> >  
> > ret = drm_dp_init_vcpi(mgr, >vcpi, pbn);
> > if (ret) {
> > -   DRM_DEBUG_KMS("failed to init vcpi %d %d %d\n", 
> > DIV_ROUND_UP(pbn, mgr->pbn_div), mgr->avail_slots, ret);
> > +   DRM_DEBUG_KMS("failed to init vcpi slots=%d max=63 ret=%d\n",
> > +   DIV_ROUND_UP(pbn, mgr->pbn_div), ret);
> > goto out;
> > }
> > -   DRM_DEBUG_KMS("initing vcpi for %d %d\n", pbn, port->vcpi.num_slots);
> > +   DRM_DEBUG_KMS("initing vcpi for pbn=%d slots=%d\n",
> > +   pbn, port->vcpi.num_slots);
> > *slots = port->vcpi.num_slots;
> >  
> > drm_dp_put_port(port);
> > diff --git a/include/drm/drm_dp_mst_helper.h 
> > b/include/drm/drm_dp_mst_helper.h
> > index 27f3e99..b0f4a09 100644
> > --- a/include/drm/drm_dp_mst_helper.h
> > +++ b/include/drm/drm_dp_mst_helper.h
> > @@ -479,18 +479,6 @@ struct drm_dp_mst_topology_mgr {
> >  * @pbn_div: PBN to slots divisor.
> >  */
> > int pbn_div;
> > -   /**
> > -* @total_slots: Total slots that can be allocated.
> > -*/
> > -   int total_slots;
> > -   /**
> > -* @avail_slots: Still available slots that can be allocated.
> > -*/
> > -   int avail_slots;
> > -   /**
> > -* @total_pbn: Total PBN count.
> > -*/
> > -   int total_pbn;
> >  
> > /**
> >  * @qlock: protects @tx_msg_downq, the tx_slots in struct
> > -- 
> > 2.7.4
> > 
> 

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

Re: [Intel-gfx] [PATCH v2 3/9] drm/dp: Split drm_dp_mst_allocate_vcpi

[Pandiyan, Dhinakaran]

On Wed, 2017-01-25 at 10:31 +1000, Dave Airlie wrote:
> On 25 January 2017 at 09:49, Dhinakaran Pandiyan
>  wrote:
> > drm_dp_mst_allocate_vcpi() apart from setting up the vcpi structure,
> > also finds if there are enough slots available. This check is a duplicate
> > of that implemented in drm_dp_mst_find_vcpi_slots(). Let's move this check
> > out and reuse the existing drm_dp_mst_find_vcpi_slots() function to check
> > if there are enough vcpi slots before allocating them.
> >
> > This brings the check to one place. Additionally drivers that will use MST
> > state tracking for atomic modesets can use the atomic version of
> > find_vcpi_slots() and reuse drm_dp_mst_allocate_vcpi()
> >
> 
> Also seem sane, at least for the core bits,
> 
> Reviewed-by: Dave Airlie 
> 

Thanks for the review.

-DK


> > Signed-off-by: Dhinakaran Pandiyan 
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c  | 20 +---
> >  drivers/gpu/drm/i915/intel_dp_mst.c|  4 ++--
> >  drivers/gpu/drm/nouveau/nv50_display.c |  3 ++-
> >  drivers/gpu/drm/radeon/radeon_dp_mst.c |  4 +++-
> >  include/drm/drm_dp_mst_helper.h|  2 +-
> >  5 files changed, 17 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index d9edd84..b871d4e 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -2479,20 +2479,17 @@ int drm_dp_find_vcpi_slots(struct 
> > drm_dp_mst_topology_mgr *mgr,
> >  EXPORT_SYMBOL(drm_dp_find_vcpi_slots);
> >
> >  static int drm_dp_init_vcpi(struct drm_dp_mst_topology_mgr *mgr,
> > -   struct drm_dp_vcpi *vcpi, int pbn)
> > +   struct drm_dp_vcpi *vcpi, int pbn, int slots)
> >  {
> > -   int num_slots;
> > int ret;
> >
> > -   num_slots = DIV_ROUND_UP(pbn, mgr->pbn_div);
> > -
> > /* max. time slots - one slot for MTP header */
> > -   if (num_slots > 63)
> > +   if (slots > 63)
> > return -ENOSPC;
> >
> > vcpi->pbn = pbn;
> > -   vcpi->aligned_pbn = num_slots * mgr->pbn_div;
> > -   vcpi->num_slots = num_slots;
> > +   vcpi->aligned_pbn = slots * mgr->pbn_div;
> > +   vcpi->num_slots = slots;
> >
> > ret = drm_dp_mst_assign_payload_id(mgr, vcpi);
> > if (ret < 0)
> > @@ -2507,7 +2504,7 @@ static int drm_dp_init_vcpi(struct 
> > drm_dp_mst_topology_mgr *mgr,
> >   * @pbn: payload bandwidth number to request
> >   * @slots: returned number of slots for this PBN.
> >   */
> > -bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr, struct 
> > drm_dp_mst_port *port, int pbn, int *slots)
> > +bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr, struct 
> > drm_dp_mst_port *port, int pbn, int slots)
> >  {
> > int ret;
> >
> > @@ -2515,16 +2512,18 @@ bool drm_dp_mst_allocate_vcpi(struct 
> > drm_dp_mst_topology_mgr *mgr, struct drm_dp
> > if (!port)
> > return false;
> >
> > +   if (slots < 0)
> > +   return false;
> > +
> > if (port->vcpi.vcpi > 0) {
> > DRM_DEBUG_KMS("payload: vcpi %d already allocated for pbn 
> > %d - requested pbn %d\n", port->vcpi.vcpi, port->vcpi.pbn, pbn);
> > if (pbn == port->vcpi.pbn) {
> > -   *slots = port->vcpi.num_slots;
> > drm_dp_put_port(port);
> > return true;
> > }
> > }
> >
> > -   ret = drm_dp_init_vcpi(mgr, >vcpi, pbn);
> > +   ret = drm_dp_init_vcpi(mgr, >vcpi, pbn, slots);
> > if (ret) {
> > DRM_DEBUG_KMS("failed to init vcpi slots=%d max=63 
> > ret=%d\n",
> > DIV_ROUND_UP(pbn, mgr->pbn_div), ret);
> > @@ -2532,7 +2531,6 @@ bool drm_dp_mst_allocate_vcpi(struct 
> > drm_dp_mst_topology_mgr *mgr, struct drm_dp
> > }
> > DRM_DEBUG_KMS("initing vcpi for pbn=%d slots=%d\n",
> > pbn, port->vcpi.num_slots);
> > -   *slots = port->vcpi.num_slots;
> >
> > drm_dp_put_port(port);
> > return true;
> > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
> > b/drivers/gpu/drm/i915/intel_dp_mst.c
> > index 38e3ca2..f51574f 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > @@ -147,7 +147,6 @@ static void intel_mst_pre_enable_dp(struct 
> > intel_encoder *encoder,
> > to_intel_connector(conn_state->connector);
> > int ret;
> > uint32_t temp;
> > -   int slots;
> >
> > /* MST encoders are bound to a crtc, not to a connector,
> >  * force the mapping here for get_hw_state.
> > @@ -177,7 +176,8 @@ static void intel_mst_pre_enable_dp(struct 
> > intel_encoder *encoder,
> >
> > ret = 

Re: [Intel-gfx] [PATCH v2 00/13] coordinate cht i2c-pmic and i915-punit accesses

[Wolfram Sang]

> So the plan (again with Wolfram's ack) is for all these patches
> including the i2c-designware ones to go upstream through the drm-intel
> tree, to avoid an intermediate state were things don't work.

Yes. I'd just like to pull in an immutable branch into my i2c/for-next
once this series is accepted.



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

Re: [Intel-gfx] [PATCH v3 14/14] drm/i915: Support explicit fencing for execbuf

[Chad Versace]

If I understand correctly, this patch preserves the kernel's current
implicit fencing, even when an input fence fd is given to execbuffer. I'm
convinced that's the right approach.

If userspace does want to disable the implicit fencing during an
execbuffer, then it should disable that explicit fencing through an
*explicit* knob. I believe the kernel should not interpret the presence
of an in fence fd in execbuffer as that knob. If it did, then using this
feature from GL/EGL userspace would be unwieldy.

In other words, I like this.

Patch 14 gets my
Acked-by: Chad Versace 

On Mon 14 Nov 2016, Chris Wilson wrote:
> Now that the user can opt-out of implicit fencing, we need to give them
> back control over the fencing. We employ sync_file to wrap our
> drm_i915_gem_request and provide an fd that userspace can merge with
> other sync_file fds and pass back to the kernel to wait upon before
> future execution.
> 
> Testcase: igt/gem_exec_fence
> Signed-off-by: Chris Wilson 
> Reviewed-by: Joonas Lahtinen 
> ---
>  drivers/gpu/drm/i915/Kconfig   |  1 +
>  drivers/gpu/drm/i915/i915_drv.c|  3 +-
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 54 
> +++---
>  include/uapi/drm/i915_drm.h| 35 ++-
>  4 files changed, 86 insertions(+), 7 deletions(-)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] drm/i915 connector list locking issues..

[Linus Torvalds]

I didn't notice this until now, because my laptop still *works*, but
there seems to be a locking issue wth the drm connector list in the
i915 driver init path.

See appended call trace.

I don't know what part of the trace is supposed to get the mode_config
locks - maybe it's the generic drm kms helpers that are broken rather
than the i915 driver.

Is nobody else seeing this? This is on my bog-standard intel laptop
(Dell XPS13 as you can see in the warning). I haven't tried to see
when it started, but it didn't happen in 4.10-rc4. So it's recent.

If I had to guess, I'd blame commit 3846fd9b8600 ("drm/probe-helpers:
Drop locking from poll_enable"). Is it just that the check is now
wrong?

Linus

---

  [drm] Found 64MB of eDRAM
  [drm] Memory usable by graphics device = 4096M
  checking generic (9000 15f9000) vs hw (9000 1000)
  fb: switching to inteldrmfb from EFI VGA
  Console: switching to colour dummy device 80x25
  [drm] Replacing VGA console driver
  [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
  [drm] Driver supports precise vblank timestamp query.
  [drm] Finished loading i915/skl_dmc_ver1_26.bin (v1.26)
  i915 :00:02.0: vgaarb: changed VGA decodes:
olddecodes=io+mem,decodes=io+mem:owns=io+mem
  [drm] GuC firmware load skipped
  [ cut here ]
  WARNING: CPU: 3 PID: 390 at ./include/drm/drm_crtc.h:857
drm_kms_helper_poll_enable.part.3+0xa8/0xc0 [drm_kms_helper]
  Modules linked in: rtsx_pci_sdmmc mmc_core crct10dif_pclmul
crc32_pclmul crc32c_intel i915(+) ghash_clmulni_intel serio_raw
i2c_algo_bit drm_kms_helper nvme rtsx_pci syscopyarea nvme_core
sysfillrect sysimgblt fb_sys_fops drm i2c_hid video fjes
  CPU: 3 PID: 390 Comm: systemd-udevd Not tainted
4.10.0-rc5-00071-ga4685d2f58e2 #10
  Hardware name: Dell Inc. XPS 13 9350/09JHRY, BIOS 1.4.12 11/30/2016
  Call Trace:
   drm_kms_helper_poll_enable.part.3+0xa8/0xc0 [drm_kms_helper]
   drm_kms_helper_poll_init+0x7e/0x90 [drm_kms_helper]
   i915_driver_load+0x13f0/0x1440 [i915]
   i915_pci_probe+0x4f/0x70 [i915]
   local_pci_probe+0x45/0xa0
   pci_device_probe+0x103/0x150
   driver_probe_device+0x2bb/0x460
   __driver_attach+0xdf/0xf0
   bus_for_each_dev+0x6c/0xc0
   driver_attach+0x1e/0x20
   bus_add_driver+0x170/0x270
   driver_register+0x60/0xe0
   __pci_register_driver+0x4c/0x50
   i915_init+0x57/0x5a [i915]
   do_one_initcall+0x52/0x1a0
   do_init_module+0x5f/0x1f8
   load_module+0x235f/0x2950
   SYSC_finit_module+0xdf/0x110
   SyS_finit_module+0xe/0x10
   do_syscall_64+0x61/0x170
   entry_SYSCALL64_slow_path+0x25/0x25
  RIP: 0033:0x7f772c536239
  RSP: 002b:7ffe3b26a3f8 EFLAGS: 0246 ORIG_RAX: 0139
  RAX: ffda RBX: 561083f72f40 RCX: 7f772c536239
  RDX:  RSI: 7f772cc3ea75 RDI: 0014
  RBP: 7f772cc3ea75 R08:  R09: 7ffe3b26a510
  R10: 0014 R11: 0246 R12: 
  R13: 561083f751d0 R14: 0002 R15: 561082b23f4a
  ---[ end trace 311d7fe73771357e ]---
  usb 1-4: new full-speed USB device number 3 using xhci_hcd
  ACPI: Video Device [GFX0] (multi-head: yes  rom: no  post: no)
  input: Video Bus as
/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/LNXVIDEO:00/input/input7
  [drm] Initialized i915 1.6.0 20161121 for :00:02.0 on minor 0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/4] drm: Resurrect atomic rmfb code, v2

[Sinclair Yeh]

On Wed, Jan 25, 2017 at 09:36:36AM +0100, Maarten Lankhorst wrote:
> Op 25-01-17 om 09:09 schreef Thomas Hellstrom:
> > On 01/25/2017 05:54 AM, Daniel Vetter wrote:
> >> On Tue, Jan 24, 2017 at 01:44:54PM -0800, Matt Roper wrote:
> >>> On Wed, Jan 11, 2017 at 05:15:47PM +0100, Daniel Vetter wrote:
>  On Thu, Dec 15, 2016 at 03:29:45PM +0100, Maarten Lankhorst wrote:
> > From: Daniel Vetter 
> >
> > This was somehow lost between v3 and the merged version in Maarten's
> > patch merged as:
> >
> > commit f2d580b9a8149735cbc4b59c4a8df60173658140
> > Author: Maarten Lankhorst 
> > Date:   Wed May 4 14:38:26 2016 +0200
> >
> > drm/core: Do not preserve framebuffer on rmfb, v4.
> >
> > Actual code copied from Maarten's patch, but with the slight change to
> > just use dev->mode_config.funcs->atomic_commit to decide whether to
> > use the atomic path or not.
> >
> > v2:
> > - Remove plane->fb assignment, done by drm_atomic_clean_old_fb.
> > - Add WARN_ON when atomic_remove_fb fails.
> > - Always call drm_atomic_state_put.
> >
> > Signed-off-by: Daniel Vetter 
> > Signed-off-by: Daniel Vetter 
> > Signed-off-by: Maarten Lankhorst 
>  Would be great if someone else could r-b this, I've proven pretty well
>  that I don't understand the complexity here :(
>  -Daniel
> >>> It looks like this will change the behavior slightly in that rmfb will
> >>> cause primary planes to be disabled, but no longer cause the entire CRTC
> >>> to be turned off.  You'll probably want to note that in the commit
> >>> message, along with the justification on why this is okay ABI-wise.
> >>>
> >>> I know that 13803132818c ("drm/core: Preserve the framebuffer after
> >>> removing it.") was initially trying to not only leave the CRTC on, but
> >>> also preserve the framebuffer and leave the planes on; that wound up
> >>> causing some kind of regression for vmwgfx, but I'm unclear on the
> >>> details there.  I'd suggest getting an Ack from one of the vmware guys
> >>> to ensure that the less drastic change in behavior here won't cause them
> >>> any problems.
> > The vmware Xorg driver is currently relying on rmfb to turn all attached
> > crtcs off. Even if we were to fix that in the Xorg driver now, older
> > Xorgs with newer kernels still would break.
> Is it allowed for vmwgfx to keep the crtc enabled, but the primary plane 
> disabled?
> 
> If so, when vmwgfx is eventually converted to atomic then we need to 
> special-case rmfb for them somehow.

FYI, we are in the process of converting things to atomic.  This may happen
around 4.12

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

Re: [Intel-gfx] DP Aux interfaces inquiry

[Ville Syrjälä]

On Wed, Jan 25, 2017 at 04:43:58PM +, mario.limoncie...@dell.com wrote:
> Thanks for your comments.  Some nested below.
> 
> > -Original Message-
> > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> > Sent: Wednesday, January 25, 2017 9:57 AM
> > To: Limonciello, Mario 
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] DP Aux interfaces inquiry
> > 
> > On Tue, Jan 24, 2017 at 07:51:28PM +, mario.limoncie...@dell.com
> > wrote:
> > > Hi,
> > >
> > > Recently Synaptics collaborated with Dell on a plugin [1] for fwupd that
> > allows flashing Synaptics MST hubs using the DP aux interface to manipulate
> > the DPCD [2].
> > > Currently the plugin hardcodes the max number of DP aux devices to look 
> > > for
> > to 3 (as that's what we've seen so far on HW), but we were wondering:
> > >
> > >   1) If there is a way to query the number of devices that the kernel is
> > going to be creating?
> > >   2) Are there any instances of more than 3 devices in the wild today that
> > anyone is aware of?
> > 
> > These depend on the board you're dealing with, and on the number of gpus you
> > have in the system (and whether they actually have drivers loaded for them).
> 
> OK, that's what I was suspecting.
> 
> > 
> > We should also perhaps expose the aux device node for MST devices. At which
> > point the number of aux nodes could change dynamically when you plug MST
> > devices in/out.
> > 
> 
> Hmm, for the devices themselves?  The way Synaptics handles cascaded MST 
> devices
> today is a remote control mechanism.  They're able to turn on remote control
> for one MST hub, and it will forward control commands (and payloads) to the 
> next
> cascaded hub.

I'm not really any kind of MST expert, so I don't really know what
people get up to with these things. But there are definitely remote DPCD
read/write messages in the spec, so I think we should be able to expose
the DPCD of any downstream device via a device node.

I took a quick stab at it:
git://github.com/vsyrjala/linux.git dp_mst_port_aux_dev

Doesn't quite seem to work though, so I probably made a mistake somewhere.

> 
> If/when you do this can you send a uevent up to userspace?  It would be good
> for fwupd to be able to listen to it and refresh devices based upon what 
> happened
> from nodes coming and going.

Hotplug uevents should be fired off whenever a new display
appears/disappears.

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 06/15] drm/doc: Clarify connector overview

[Gustavo Padovan]

2017-01-25 Daniel Vetter :

> On Wed, Jan 25, 2017 at 10:57:17AM -0200, Gustavo Padovan wrote:
> > Hi Daniel,
> > 
> > 2017-01-25 Daniel Vetter :
> > 
> > > There was a bit of mix-up between initialization and registering.
> > > 
> > > Signed-off-by: Daniel Vetter 
> > > ---
> > >  drivers/gpu/drm/drm_connector.c | 9 -
> > >  1 file changed, 4 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_connector.c 
> > > b/drivers/gpu/drm/drm_connector.c
> > > index dd720d4cb4f7..c75ab242f907 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -40,11 +40,10 @@
> > >   *
> > >   * KMS driver must create, initialize, register and attach at a 
> > >   * drm_connector for each such sink. The instance is created as other KMS
> > > - * objects and initialized by setting the following fields.
> > > - *
> > > - * The connector is then registered with a call to drm_connector_init() 
> > > with a
> > > - * pointer to the connector functions and a connector type, and exposed 
> > > through
> > > - * sysfs with a call to drm_connector_register().
> > > + * objects and initialized by setting the following fields. The 
> > > connector is
> > > + * initialized with a call to drm_connector_init() with a pointer to the
> > > + * connector functions and a connector type, and then exposed to 
> > > userspace with
> > 
> > _connector_funcs
> 
> I'm not really clear what you want me to do here ...

You said "a pointer to the connector functions" thus
_connector_funcs. Sorry for not being clear enough, I should have 
justified what I wrote.

> -Daniel
> 
> > 
> > Other than that:
> > 
> > Reviewed-by: Gustavo Padovan 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 12/15] drm/moc: Mark legacy fields in drm_driver as such

[Alex Deucher]

On Wed, Jan 25, 2017 at 1:26 AM, Daniel Vetter  wrote:
> No point in documenting these, they only confuse.
>
> Signed-off-by: Daniel Vetter 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  1 -
>  include/drm/drm_drv.h   | 13 -
>  2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 2534adaebe30..38c22c3c0d54 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -686,7 +686,6 @@ static struct drm_driver kms_driver = {
> DRIVER_USE_AGP |
> DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM |
> DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET,
> -   .dev_priv_size = 0,
> .load = amdgpu_driver_load_kms,
> .open = amdgpu_driver_open_kms,
> .preclose = amdgpu_driver_preclose_kms,
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 8391135b95f2..732e85652d1e 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -81,7 +81,6 @@ struct drm_driver {
>  * Zero on success, non-zero value on failure.
>  */
> int (*load) (struct drm_device *, unsigned long flags);
> -   int (*firstopen) (struct drm_device *);
> int (*open) (struct drm_device *, struct drm_file *);
> void (*preclose) (struct drm_device *, struct drm_file *file_priv);
> void (*postclose) (struct drm_device *, struct drm_file *);
> @@ -103,9 +102,6 @@ struct drm_driver {
>  *
>  */
> void (*unload) (struct drm_device *);
> -   int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file 
> *file_priv);
> -   int (*dma_quiescent) (struct drm_device *);
> -   int (*context_dtor) (struct drm_device *dev, int context);
> int (*set_busid)(struct drm_device *dev, struct drm_master *master);
>
> /**
> @@ -413,13 +409,20 @@ struct drm_driver {
> char *date;
>
> u32 driver_features;
> -   int dev_priv_size;
> const struct drm_ioctl_desc *ioctls;
> int num_ioctls;
> const struct file_operations *fops;
>
> +   /* Everything below here is for legacy driver, never use! */
> +   /* private: */
> +
> /* List of devices hanging off this driver with stealth attach. */
> struct list_head legacy_dev_list;
> +   int (*firstopen) (struct drm_device *);
> +   int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file 
> *file_priv);
> +   int (*dma_quiescent) (struct drm_device *);
> +   int (*context_dtor) (struct drm_device *dev, int context);
> +   int dev_priv_size;
>  };
>
>  extern __printf(6, 7)
> --
> 2.11.0
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 11/15] drm: Nuke ums vgaarb support

[Alex Deucher]

On Wed, Jan 25, 2017 at 1:26 AM, Daniel Vetter  wrote:
> i915, nouveau (ever since merged to upstream) and radeon all lack ums
> support in upstream. No point keeping the ums vgaarb support around.
>
> Signed-off-by: Daniel Vetter 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/drm_irq.c | 26 --
>  include/drm/drm_drv.h |  3 ---
>  2 files changed, 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 1c4da043eeda..954960cc68e7 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -415,29 +415,6 @@ int drm_vblank_init(struct drm_device *dev, unsigned int 
> num_crtcs)
>  }
>  EXPORT_SYMBOL(drm_vblank_init);
>
> -static void drm_irq_vgaarb_nokms(void *cookie, bool state)
> -{
> -   struct drm_device *dev = cookie;
> -
> -   if (dev->driver->vgaarb_irq) {
> -   dev->driver->vgaarb_irq(dev, state);
> -   return;
> -   }
> -
> -   if (!dev->irq_enabled)
> -   return;
> -
> -   if (state) {
> -   if (dev->driver->irq_uninstall)
> -   dev->driver->irq_uninstall(dev);
> -   } else {
> -   if (dev->driver->irq_preinstall)
> -   dev->driver->irq_preinstall(dev);
> -   if (dev->driver->irq_postinstall)
> -   dev->driver->irq_postinstall(dev);
> -   }
> -}
> -
>  /**
>   * drm_irq_install - install IRQ handler
>   * @dev: DRM device
> @@ -492,9 +469,6 @@ int drm_irq_install(struct drm_device *dev, int irq)
> return ret;
> }
>
> -   if (drm_core_check_feature(dev, DRIVER_LEGACY))
> -   vga_client_register(dev->pdev, (void *)dev, 
> drm_irq_vgaarb_nokms, NULL);
> -
> /* After installing handler */
> if (dev->driver->irq_postinstall)
> ret = dev->driver->irq_postinstall(dev);
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index af75fc6ec830..8391135b95f2 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -345,9 +345,6 @@ struct drm_driver {
> int (*gem_prime_mmap)(struct drm_gem_object *obj,
> struct vm_area_struct *vma);
>
> -   /* vga arb irq handler */
> -   void (*vgaarb_irq)(struct drm_device *dev, bool state);
> -
> /**
>  * @dumb_create:
>  *
> --
> 2.11.0
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 08/15] drm/i810: drop device_is_agp callback

[Alex Deucher]

On Wed, Jan 25, 2017 at 1:26 AM, Daniel Vetter  wrote:
> Use the same trick we used for i915 when we still had ums support:
> Just initialize the agp support unconditionally in the driver load
> function.
>
> Unfortunately that means we need to export drm_agp_init again, but I
> think that's a lesser evil.
>
> Signed-off-by: Daniel Vetter 

Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/drm_agpsupport.c |  2 ++
>  drivers/gpu/drm/i810/i810_dma.c  | 24 
>  drivers/gpu/drm/i810/i810_drv.c  |  1 -
>  drivers/gpu/drm/i810/i810_drv.h  |  1 -
>  4 files changed, 10 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_agpsupport.c 
> b/drivers/gpu/drm/drm_agpsupport.c
> index d621c8a4cf00..c89953449e96 100644
> --- a/drivers/gpu/drm/drm_agpsupport.c
> +++ b/drivers/gpu/drm/drm_agpsupport.c
> @@ -421,6 +421,8 @@ struct drm_agp_head *drm_agp_init(struct drm_device *dev)
> head->base = head->agp_info.aper_base;
> return head;
>  }
> +/* Only exported for i810.ko */
> +EXPORT_SYMBOL(drm_agp_init);
>
>  /**
>   * drm_legacy_agp_clear - Clear AGP resource list
> diff --git a/drivers/gpu/drm/i810/i810_dma.c b/drivers/gpu/drm/i810/i810_dma.c
> index ab4e6cbe1f8b..576a417690d4 100644
> --- a/drivers/gpu/drm/i810/i810_dma.c
> +++ b/drivers/gpu/drm/i810/i810_dma.c
> @@ -1190,6 +1190,14 @@ static int i810_flip_bufs(struct drm_device *dev, void 
> *data,
>
>  int i810_driver_load(struct drm_device *dev, unsigned long flags)
>  {
> +   dev->agp = drm_agp_init(dev);
> +   if (dev->agp) {
> +   dev->agp->agp_mtrr = arch_phys_wc_add(
> +   dev->agp->agp_info.aper_base,
> +   dev->agp->agp_info.aper_size *
> +   1024 * 1024);
> +   }
> +
> /* Our userspace depends upon the agp mapping support. */
> if (!dev->agp)
> return -EINVAL;
> @@ -1249,19 +1257,3 @@ const struct drm_ioctl_desc i810_ioctls[] = {
>  };
>
>  int i810_max_ioctl = ARRAY_SIZE(i810_ioctls);
> -
> -/**
> - * Determine if the device really is AGP or not.
> - *
> - * All Intel graphics chipsets are treated as AGP, even if they are really
> - * PCI-e.
> - *
> - * \param dev   The device to be tested.
> - *
> - * \returns
> - * A value of 1 is always retured to indictate every i810 is AGP.
> - */
> -int i810_driver_device_is_agp(struct drm_device *dev)
> -{
> -   return 1;
> -}
> diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c
> index 02504a7cfaf2..37fd0906f807 100644
> --- a/drivers/gpu/drm/i810/i810_drv.c
> +++ b/drivers/gpu/drm/i810/i810_drv.c
> @@ -60,7 +60,6 @@ static struct drm_driver driver = {
> .lastclose = i810_driver_lastclose,
> .preclose = i810_driver_preclose,
> .set_busid = drm_pci_set_busid,
> -   .device_is_agp = i810_driver_device_is_agp,
> .dma_quiescent = i810_driver_dma_quiescent,
> .ioctls = i810_ioctls,
> .fops = _driver_fops,
> diff --git a/drivers/gpu/drm/i810/i810_drv.h b/drivers/gpu/drm/i810/i810_drv.h
> index 93ec5dc4e7d3..c73d2f2da57b 100644
> --- a/drivers/gpu/drm/i810/i810_drv.h
> +++ b/drivers/gpu/drm/i810/i810_drv.h
> @@ -124,7 +124,6 @@ extern int i810_driver_load(struct drm_device *, unsigned 
> long flags);
>  extern void i810_driver_lastclose(struct drm_device *dev);
>  extern void i810_driver_preclose(struct drm_device *dev,
>  struct drm_file *file_priv);
> -extern int i810_driver_device_is_agp(struct drm_device *dev);
>
>  extern long i810_ioctl(struct file *file, unsigned int cmd, unsigned long 
> arg);
>  extern const struct drm_ioctl_desc i810_ioctls[];
> --
> 2.11.0
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 10/15] drm: remove device_is_agp callback

[Alex Deucher]

On Wed, Jan 25, 2017 at 1:26 AM, Daniel Vetter  wrote:
> With that the drm_pci_device_is_agp function becomes trivial, so
> inline that too. And while at it, move the drm_pci_agp_destroy
> declaration into drm-internal.h, since it's not used by drivers.
>
> Cc: Alex Deucher 
> Cc: Ben Skeggs 
> Signed-off-by: Daniel Vetter 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/drm_internal.h  |  1 +
>  drivers/gpu/drm/drm_pci.c   |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_abi16.c |  2 +-
>  drivers/gpu/drm/radeon/radeon_cs.c  |  3 ++-
>  drivers/gpu/drm/radeon/radeon_kms.c |  2 +-
>  include/drm/drmP.h  | 15 ---
>  include/drm/drm_drv.h   | 14 --
>  7 files changed, 6 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index a6213f814345..f37388cb2fde 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -31,6 +31,7 @@ void drm_lastclose(struct drm_device *dev);
>  /* drm_pci.c */
>  int drm_irq_by_busid(struct drm_device *dev, void *data,
>  struct drm_file *file_priv);
> +void drm_pci_agp_destroy(struct drm_device *dev);
>
>  /* drm_prime.c */
>  int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index 50c732a95b5a..b347c92914cf 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -191,7 +191,7 @@ int drm_irq_by_busid(struct drm_device *dev, void *data,
>  static void drm_pci_agp_init(struct drm_device *dev)
>  {
> if (drm_core_check_feature(dev, DRIVER_USE_AGP)) {
> -   if (drm_pci_device_is_agp(dev))
> +   if (pci_find_capability(dev->pdev, PCI_CAP_ID_AGP))
> dev->agp = drm_agp_init(dev);
> if (dev->agp) {
> dev->agp->agp_mtrr = arch_phys_wc_add(
> diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c 
> b/drivers/gpu/drm/nouveau/nouveau_abi16.c
> index 7bd4683216d0..4df4f6ed4886 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
> @@ -199,7 +199,7 @@ nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS)
> if (!nvxx_device(device)->func->pci)
> getparam->value = 3;
> else
> -   if (drm_pci_device_is_agp(dev))
> +   if (pci_find_capability(dev->pdev, PCI_CAP_ID_AGP))
> getparam->value = 0;
> else
> if (!pci_is_pcie(dev->pdev))
> diff --git a/drivers/gpu/drm/radeon/radeon_cs.c 
> b/drivers/gpu/drm/radeon/radeon_cs.c
> index 510ea371dacc..a8442f7196d6 100644
> --- a/drivers/gpu/drm/radeon/radeon_cs.c
> +++ b/drivers/gpu/drm/radeon/radeon_cs.c
> @@ -121,7 +121,8 @@ static int radeon_cs_parser_relocs(struct 
> radeon_cs_parser *p)
>VRAM, also but everything into VRAM on AGP cards and older
>IGP chips to avoid image corruptions */
> if (p->ring == R600_RING_TYPE_UVD_INDEX &&
> -   (i == 0 || drm_pci_device_is_agp(p->rdev->ddev) ||
> +   (i == 0 || pci_find_capability(p->rdev->ddev->pdev,
> +  PCI_CAP_ID_AGP) ||
>  p->rdev->family == CHIP_RS780 ||
>  p->rdev->family == CHIP_RS880)) {
>
> diff --git a/drivers/gpu/drm/radeon/radeon_kms.c 
> b/drivers/gpu/drm/radeon/radeon_kms.c
> index 116cf0d23595..56f35c06742c 100644
> --- a/drivers/gpu/drm/radeon/radeon_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_kms.c
> @@ -105,7 +105,7 @@ int radeon_driver_load_kms(struct drm_device *dev, 
> unsigned long flags)
> dev->dev_private = (void *)rdev;
>
> /* update BUS flag */
> -   if (drm_pci_device_is_agp(dev)) {
> +   if (pci_find_capability(dev->pdev, PCI_CAP_ID_AGP)) {
> flags |= RADEON_IS_AGP;
> } else if (pci_is_pcie(dev->pdev)) {
> flags |= RADEON_IS_PCIE;
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index e5882d5a68e5..21a3a666a2fd 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -790,21 +790,6 @@ extern void drm_sysfs_hotplug_event(struct drm_device 
> *dev);
>
>  /*@}*/
>
> -/* PCI section */
> -static __inline__ int drm_pci_device_is_agp(struct drm_device *dev)
> -{
> -   if (dev->driver->device_is_agp != NULL) {
> -   int err = (*dev->driver->device_is_agp) (dev);
> -
> -   if (err != 2) {
> -   return err;
> -   }
> -   }
> -
> -   return pci_find_capability(dev->pdev, PCI_CAP_ID_AGP);
> -}
> -void drm_pci_agp_destroy(struct drm_device *dev);
> -
>  extern int 

Re: [Intel-gfx] [PATCH 09/15] drm/mga: remove device_is_agp callback

[Alex Deucher]

On Wed, Jan 25, 2017 at 1:26 AM, Daniel Vetter  wrote:
> It's only for a device quirk, and we might as well do that in the load
> callback.
>
> Signed-off-by: Daniel Vetter 

Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/mga/mga_dma.c | 20 +++-
>  drivers/gpu/drm/mga/mga_drv.c | 37 -
>  2 files changed, 19 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/mga/mga_dma.c b/drivers/gpu/drm/mga/mga_dma.c
> index a1d8dd15b131..1ffdafea27e4 100644
> --- a/drivers/gpu/drm/mga/mga_dma.c
> +++ b/drivers/gpu/drm/mga/mga_dma.c
> @@ -392,6 +392,24 @@ int mga_driver_load(struct drm_device *dev, unsigned 
> long flags)
> drm_mga_private_t *dev_priv;
> int ret;
>
> +   /* There are PCI versions of the G450.  These cards have the
> +* same PCI ID as the AGP G450, but have an additional PCI-to-PCI
> +* bridge chip.  We detect these cards, which are not currently
> +* supported by this driver, by looking at the device ID of the
> +* bus the "card" is on.  If vendor is 0x3388 (Hint Corp) and the
> +* device is 0x0021 (HB6 Universal PCI-PCI bridge), we reject the
> +* device.
> +*/
> +   if ((dev->pdev->device == 0x0525) && dev->pdev->bus->self
> +   && (dev->pdev->bus->self->vendor == 0x3388)
> +   && (dev->pdev->bus->self->device == 0x0021)
> +   && dev->agp) {
> +   /* FIXME: This should be quirked in the pci core, but oh well
> +* the hw probably stopped existing. */
> +   arch_phys_wc_del(dev->agp->agp_mtrr);
> +   kfree(dev->agp);
> +   dev->agp = NULL;
> +   }
> dev_priv = kzalloc(sizeof(drm_mga_private_t), GFP_KERNEL);
> if (!dev_priv)
> return -ENOMEM;
> @@ -698,7 +716,7 @@ static int mga_do_pci_dma_bootstrap(struct drm_device 
> *dev,
>  static int mga_do_dma_bootstrap(struct drm_device *dev,
> drm_mga_dma_bootstrap_t *dma_bs)
>  {
> -   const int is_agp = (dma_bs->agp_mode != 0) && 
> drm_pci_device_is_agp(dev);
> +   const int is_agp = (dma_bs->agp_mode != 0) && dev->agp;
> int err;
> drm_mga_private_t *const dev_priv =
> (drm_mga_private_t *) dev->dev_private;
> diff --git a/drivers/gpu/drm/mga/mga_drv.c b/drivers/gpu/drm/mga/mga_drv.c
> index 25b2a1a424e6..63ba0699d107 100644
> --- a/drivers/gpu/drm/mga/mga_drv.c
> +++ b/drivers/gpu/drm/mga/mga_drv.c
> @@ -37,8 +37,6 @@
>
>  #include 
>
> -static int mga_driver_device_is_agp(struct drm_device *dev);
> -
>  static struct pci_device_id pciidlist[] = {
> mga_PCI_IDS
>  };
> @@ -66,7 +64,6 @@ static struct drm_driver driver = {
> .lastclose = mga_driver_lastclose,
> .set_busid = drm_pci_set_busid,
> .dma_quiescent = mga_driver_dma_quiescent,
> -   .device_is_agp = mga_driver_device_is_agp,
> .get_vblank_counter = mga_get_vblank_counter,
> .enable_vblank = mga_enable_vblank,
> .disable_vblank = mga_disable_vblank,
> @@ -107,37 +104,3 @@ module_exit(mga_exit);
>  MODULE_AUTHOR(DRIVER_AUTHOR);
>  MODULE_DESCRIPTION(DRIVER_DESC);
>  MODULE_LICENSE("GPL and additional rights");
> -
> -/**
> - * Determine if the device really is AGP or not.
> - *
> - * In addition to the usual tests performed by \c drm_device_is_agp, this
> - * function detects PCI G450 cards that appear to the system exactly like
> - * AGP G450 cards.
> - *
> - * \param dev   The device to be tested.
> - *
> - * \returns
> - * If the device is a PCI G450, zero is returned.  Otherwise 2 is returned.
> - */
> -static int mga_driver_device_is_agp(struct drm_device *dev)
> -{
> -   const struct pci_dev *const pdev = dev->pdev;
> -
> -   /* There are PCI versions of the G450.  These cards have the
> -* same PCI ID as the AGP G450, but have an additional PCI-to-PCI
> -* bridge chip.  We detect these cards, which are not currently
> -* supported by this driver, by looking at the device ID of the
> -* bus the "card" is on.  If vendor is 0x3388 (Hint Corp) and the
> -* device is 0x0021 (HB6 Universal PCI-PCI bridge), we reject the
> -* device.
> -*/
> -
> -   if ((pdev->device == 0x0525) && pdev->bus->self
> -   && (pdev->bus->self->vendor == 0x3388)
> -   && (pdev->bus->self->device == 0x0021)) {
> -   return 0;
> -   }
> -
> -   return 2;
> -}
> --
> 2.11.0
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 14/15] drm: s/drm_crtc_get_hv_timings/drm_mode_get_hv_timings/

[Alex Deucher]

On Wed, Jan 25, 2017 at 1:26 AM, Daniel Vetter  wrote:
> The function operates on modes, not CRTCs. Also move it into
> drm_modes.[hc]. Spotted while reviewing CRTC docs.
>
> Signed-off-by: Daniel Vetter 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/drm_atomic_helper.c  |  2 +-
>  drivers/gpu/drm/drm_crtc.c   | 23 +--
>  drivers/gpu/drm/drm_modes.c  | 21 +
>  drivers/gpu/drm/i915/intel_display.c |  4 ++--
>  include/drm/drm_crtc.h   |  2 --
>  include/drm/drm_modes.h  |  2 ++
>  6 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 5e512dd3a2c4..9633d12c4ed1 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2369,7 +2369,7 @@ int __drm_atomic_helper_set_config(struct drm_mode_set 
> *set,
> if (ret != 0)
> return ret;
>
> -   drm_crtc_get_hv_timing(set->mode, , );
> +   drm_mode_get_hv_timing(set->mode, , );
>
> drm_atomic_set_fb_for_plane(primary_state, set->fb);
> primary_state->crtc_x = 0;
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index cea7a7efa43c..5f28e3a5a3e0 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -461,27 +461,6 @@ int drm_mode_set_config_internal(struct drm_mode_set 
> *set)
>  EXPORT_SYMBOL(drm_mode_set_config_internal);
>
>  /**
> - * drm_crtc_get_hv_timing - Fetches hdisplay/vdisplay for given mode
> - * @mode: mode to query
> - * @hdisplay: hdisplay value to fill in
> - * @vdisplay: vdisplay value to fill in
> - *
> - * The vdisplay value will be doubled if the specified mode is a stereo mode 
> of
> - * the appropriate layout.
> - */
> -void drm_crtc_get_hv_timing(const struct drm_display_mode *mode,
> -   int *hdisplay, int *vdisplay)
> -{
> -   struct drm_display_mode adjusted;
> -
> -   drm_mode_copy(, mode);
> -   drm_mode_set_crtcinfo(, CRTC_STEREO_DOUBLE_ONLY);
> -   *hdisplay = adjusted.crtc_hdisplay;
> -   *vdisplay = adjusted.crtc_vdisplay;
> -}
> -EXPORT_SYMBOL(drm_crtc_get_hv_timing);
> -
> -/**
>   * drm_crtc_check_viewport - Checks that a framebuffer is big enough for the
>   * CRTC viewport
>   * @crtc: CRTC that framebuffer will be displayed on
> @@ -498,7 +477,7 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,
>  {
> int hdisplay, vdisplay;
>
> -   drm_crtc_get_hv_timing(mode, , );
> +   drm_mode_get_hv_timing(mode, , );
>
> if (crtc->state &&
> drm_rotation_90_or_270(crtc->primary->state->rotation))
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index e6b19bc9021a..73ed6399c3fb 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -797,6 +797,27 @@ int drm_mode_vrefresh(const struct drm_display_mode 
> *mode)
>  EXPORT_SYMBOL(drm_mode_vrefresh);
>
>  /**
> + * drm_mode_get_hv_timing - Fetches hdisplay/vdisplay for given mode
> + * @mode: mode to query
> + * @hdisplay: hdisplay value to fill in
> + * @vdisplay: vdisplay value to fill in
> + *
> + * The vdisplay value will be doubled if the specified mode is a stereo mode 
> of
> + * the appropriate layout.
> + */
> +void drm_mode_get_hv_timing(const struct drm_display_mode *mode,
> +   int *hdisplay, int *vdisplay)
> +{
> +   struct drm_display_mode adjusted;
> +
> +   drm_mode_copy(, mode);
> +   drm_mode_set_crtcinfo(, CRTC_STEREO_DOUBLE_ONLY);
> +   *hdisplay = adjusted.crtc_hdisplay;
> +   *vdisplay = adjusted.crtc_vdisplay;
> +}
> +EXPORT_SYMBOL(drm_mode_get_hv_timing);
> +
> +/**
>   * drm_mode_set_crtcinfo - set CRTC modesetting timing parameters
>   * @p: mode
>   * @adjust_flags: a combination of adjustment flags
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 7af8ddcaa075..50096f9bc420 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11099,7 +11099,7 @@ static int intel_modeset_setup_plane_state(struct 
> drm_atomic_state *state,
> return PTR_ERR(plane_state);
>
> if (mode)
> -   drm_crtc_get_hv_timing(mode, , );
> +   drm_mode_get_hv_timing(mode, , );
> else
> hdisplay = vdisplay = 0;
>
> @@ -12992,7 +12992,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>  * computation to clearly distinguish it from the adjusted mode, which
>  * can be changed by the connectors in the below retry loop.
>  */
> -   drm_crtc_get_hv_timing(_config->base.mode,
> +   drm_mode_get_hv_timing(_config->base.mode,
>_config->pipe_src_w,
>

Re: [Intel-gfx] DP Aux interfaces inquiry

[Ville Syrjälä]

On Wed, Jan 25, 2017 at 09:13:44AM -0800, Rafael Antognolli wrote:
> Hi Mario, please see below...
> 
> On Wed, Jan 25, 2017 at 04:43:58PM +, mario.limoncie...@dell.com wrote:
> > Thanks for your comments.  Some nested below.
> > 
> > > -Original Message-
> > > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> > > Sent: Wednesday, January 25, 2017 9:57 AM
> > > To: Limonciello, Mario 
> > > Cc: intel-gfx@lists.freedesktop.org
> > > Subject: Re: [Intel-gfx] DP Aux interfaces inquiry
> > > 
> > > On Tue, Jan 24, 2017 at 07:51:28PM +, mario.limoncie...@dell.com
> > > wrote:
> > > > Hi,
> > > >
> > > > Recently Synaptics collaborated with Dell on a plugin [1] for fwupd that
> > > allows flashing Synaptics MST hubs using the DP aux interface to 
> > > manipulate
> > > the DPCD [2].
> > > > Currently the plugin hardcodes the max number of DP aux devices to look 
> > > > for
> > > to 3 (as that's what we've seen so far on HW), but we were wondering:
> > > >
> > > > 1) If there is a way to query the number of devices that the 
> > > > kernel is
> > > going to be creating?
> > > > 2) Are there any instances of more than 3 devices in the wild 
> > > > today that
> > > anyone is aware of?
> > > 
> > > These depend on the board you're dealing with, and on the number of gpus 
> > > you
> > > have in the system (and whether they actually have drivers loaded for 
> > > them).
> > 
> > OK, that's what I was suspecting.
> > 
> > > 
> > > We should also perhaps expose the aux device node for MST devices. At 
> > > which
> > > point the number of aux nodes could change dynamically when you plug MST
> > > devices in/out.
> > > 
> > 
> > Hmm, for the devices themselves?  The way Synaptics handles cascaded MST 
> > devices
> > today is a remote control mechanism.  They're able to turn on remote control
> > for one MST hub, and it will forward control commands (and payloads) to the 
> > next
> > cascaded hub.
> > 
> > If/when you do this can you send a uevent up to userspace?  It would be good
> > for fwupd to be able to listen to it and refresh devices based upon what 
> > happened
> > from nodes coming and going.
> > 
> > > So I don't think you should be making any assumptions on the number/order 
> > > of
> > > these device nodes.
> > > 
> > 
> > From userspace would it be better to just scan /dev for /dev/drm_dp_aux# 
> > nodes?
> > No assumptions about the order of them, just look for all the ones with 
> > that prefix?
> > 
> > If not, do you have a better recommendation on how to do this?
> 
> Alternatively, you can also scan /sys/class/drm_dp_aux_dev/. The number
> of the files that you have there should match what you have inside /dev.
> It's not necessarily better though.

Or if you already know the connector that you want to use, then you can
find the sysfs stuff for the correct aux device directly under the
connector somewhere in /sys/class/drm/

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Remove early pre-production RPS workarounds for BXT

[Chris Wilson]

Remove WaGsvDisableTurbo and WaRsUseTimeoutMode as these were only for
pre-production Broxton devices, and this code is now defunct.

Signed-off-by: Chris Wilson 
Cc: Mika Kuoppala 
Cc: Joonas Lahtinen 
---
 drivers/gpu/drm/i915/intel_pm.c | 35 +++
 1 file changed, 3 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a58c0edd7578..6cf3fb8e12cc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4935,10 +4935,6 @@ static u32 gen6_rps_pm_mask(struct drm_i915_private 
*dev_priv, u8 val)
  * update the GEN6_RP_INTERRUPT_LIMITS register accordingly. */
 static int gen6_set_rps(struct drm_i915_private *dev_priv, u8 val)
 {
-   /* WaGsvDisableTurbo: Workaround to disable turbo on BXT A* */
-   if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_A1))
-   return 0;
-
WARN_ON(!mutex_is_locked(_priv->rps.hw_lock));
WARN_ON(val > dev_priv->rps.max_freq);
WARN_ON(val < dev_priv->rps.min_freq);
@@ -5353,22 +5349,6 @@ static void gen9_enable_rps(struct drm_i915_private 
*dev_priv)
 {
intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
 
-   /* WaGsvDisableTurbo: Workaround to disable turbo on BXT A* */
-   if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_A1)) {
-   /*
-* BIOS could leave the Hw Turbo enabled, so need to explicitly
-* clear out the Control register just to avoid inconsitency
-* with debugfs interface, which will show  Turbo as enabled
-* only and that is not expected by the User after adding the
-* WaGsvDisableTurbo. Apart from this there is no problem even
-* if the Turbo is left enabled in the Control register, as the
-* Up/Down interrupts would remain masked.
-*/
-   gen9_disable_rps(dev_priv);
-   intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
-   return;
-   }
-
/* Program defaults and thresholds for RPS*/
I915_WRITE(GEN6_RC_VIDEO_FREQ,
GEN9_FREQUENCY(dev_priv->rps.rp1_freq));
@@ -5428,18 +5408,9 @@ static void gen9_enable_rc6(struct drm_i915_private 
*dev_priv)
if (intel_enable_rc6() & INTEL_RC6_ENABLE)
rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
DRM_INFO("RC6 %s\n", onoff(rc6_mask & GEN6_RC_CTL_RC6_ENABLE));
-   /* WaRsUseTimeoutMode:bxt */
-   if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_A1)) {
-   I915_WRITE(GEN6_RC6_THRESHOLD, 625); /* 800us */
-   I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
-  GEN7_RC_CTL_TO_MODE |
-  rc6_mask);
-   } else {
-   I915_WRITE(GEN6_RC6_THRESHOLD, 37500); /* 37.5/125ms per EI */
-   I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
-  GEN6_RC_CTL_EI_MODE(1) |
-  rc6_mask);
-   }
+   I915_WRITE(GEN6_RC6_THRESHOLD, 37500); /* 37.5/125ms per EI */
+   I915_WRITE(GEN6_RC_CONTROL,
+  GEN6_RC_CTL_HW_ENABLE | GEN6_RC_CTL_EI_MODE(1) | rc6_mask);
 
/*
 * 3b: Enable Coarse Power Gating only when RC6 is enabled.
-- 
2.11.0

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

Re: [Intel-gfx] DP Aux interfaces inquiry

[Rafael Antognolli]

Hi Mario, please see below...

On Wed, Jan 25, 2017 at 04:43:58PM +, mario.limoncie...@dell.com wrote:
> Thanks for your comments.  Some nested below.
> 
> > -Original Message-
> > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> > Sent: Wednesday, January 25, 2017 9:57 AM
> > To: Limonciello, Mario 
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] DP Aux interfaces inquiry
> > 
> > On Tue, Jan 24, 2017 at 07:51:28PM +, mario.limoncie...@dell.com
> > wrote:
> > > Hi,
> > >
> > > Recently Synaptics collaborated with Dell on a plugin [1] for fwupd that
> > allows flashing Synaptics MST hubs using the DP aux interface to manipulate
> > the DPCD [2].
> > > Currently the plugin hardcodes the max number of DP aux devices to look 
> > > for
> > to 3 (as that's what we've seen so far on HW), but we were wondering:
> > >
> > >   1) If there is a way to query the number of devices that the kernel is
> > going to be creating?
> > >   2) Are there any instances of more than 3 devices in the wild today that
> > anyone is aware of?
> > 
> > These depend on the board you're dealing with, and on the number of gpus you
> > have in the system (and whether they actually have drivers loaded for them).
> 
> OK, that's what I was suspecting.
> 
> > 
> > We should also perhaps expose the aux device node for MST devices. At which
> > point the number of aux nodes could change dynamically when you plug MST
> > devices in/out.
> > 
> 
> Hmm, for the devices themselves?  The way Synaptics handles cascaded MST 
> devices
> today is a remote control mechanism.  They're able to turn on remote control
> for one MST hub, and it will forward control commands (and payloads) to the 
> next
> cascaded hub.
> 
> If/when you do this can you send a uevent up to userspace?  It would be good
> for fwupd to be able to listen to it and refresh devices based upon what 
> happened
> from nodes coming and going.
> 
> > So I don't think you should be making any assumptions on the number/order of
> > these device nodes.
> > 
> 
> From userspace would it be better to just scan /dev for /dev/drm_dp_aux# 
> nodes?
> No assumptions about the order of them, just look for all the ones with that 
> prefix?
> 
> If not, do you have a better recommendation on how to do this?

Alternatively, you can also scan /sys/class/drm_dp_aux_dev/. The number
of the files that you have there should match what you have inside /dev.
It's not necessarily better though.

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

Re: [Intel-gfx] DP Aux interfaces inquiry

[Mario.Limonciello]

Thanks for your comments.  Some nested below.

> -Original Message-
> From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> Sent: Wednesday, January 25, 2017 9:57 AM
> To: Limonciello, Mario 
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] DP Aux interfaces inquiry
> 
> On Tue, Jan 24, 2017 at 07:51:28PM +, mario.limoncie...@dell.com
> wrote:
> > Hi,
> >
> > Recently Synaptics collaborated with Dell on a plugin [1] for fwupd that
> allows flashing Synaptics MST hubs using the DP aux interface to manipulate
> the DPCD [2].
> > Currently the plugin hardcodes the max number of DP aux devices to look for
> to 3 (as that's what we've seen so far on HW), but we were wondering:
> >
> > 1) If there is a way to query the number of devices that the kernel is
> going to be creating?
> > 2) Are there any instances of more than 3 devices in the wild today that
> anyone is aware of?
> 
> These depend on the board you're dealing with, and on the number of gpus you
> have in the system (and whether they actually have drivers loaded for them).

OK, that's what I was suspecting.

> 
> We should also perhaps expose the aux device node for MST devices. At which
> point the number of aux nodes could change dynamically when you plug MST
> devices in/out.
> 

Hmm, for the devices themselves?  The way Synaptics handles cascaded MST devices
today is a remote control mechanism.  They're able to turn on remote control
for one MST hub, and it will forward control commands (and payloads) to the next
cascaded hub.

If/when you do this can you send a uevent up to userspace?  It would be good
for fwupd to be able to listen to it and refresh devices based upon what 
happened
from nodes coming and going.

> So I don't think you should be making any assumptions on the number/order of
> these device nodes.
> 

From userspace would it be better to just scan /dev for /dev/drm_dp_aux# nodes?
No assumptions about the order of them, just look for all the ones with that 
prefix?

If not, do you have a better recommendation on how to do this?

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

Re: [Intel-gfx] [PATCH 5/5] drm/i915/perf: improve tail race workaround

[Matthew Auld]

On 24 January 2017 at 01:25, Robert Bragg  wrote:
> There's a HW race condition between OA unit tail pointer register
> updates and writes to memory whereby the tail pointer can sometimes get
> ahead of what's been written out to the OA buffer so far (in terms of
> what's visible to the CPU).
>
> Although this can be observed explicitly while copying reports to
> userspace by checking for a zeroed report-id field in tail reports, we
> want to account for this earlier, as part of the _oa_buffer_check to
> avoid lots of redundant read() attempts.
>
> Previously the driver used to define an effective tail pointer that
> lagged the real pointer by a 'tail margin' measured in bytes derived
> from OA_TAIL_MARGIN_NSEC and the configured sampling frequency.
> Unfortunately this was flawed considering that the OA unit may also
> automatically generate non-periodic reports (such as on context switch)
> or the OA unit may be enabled without any periodic sampling.
>
> This improves how we define a tail pointer for reading that lags the
> real tail pointer by at least %OA_TAIL_MARGIN_NSEC nanoseconds, which
> gives enough time for the corresponding reports to become visible to the
> CPU.
>
> The driver now maintains two tail pointers:
>  1) An 'aging' tail with an associated timestamp that is tracked until we
> can trust the corresponding data is visible to the CPU; at which point
> it is considered 'aged'.
>  2) An 'aged' tail that can be used for read()ing.
>
> The two separate pointers let us decouple read()s from tail pointer aging.
>
> The tail pointers are checked and updated at a limited rate within a
> hrtimer callback (the same callback that is used for delivering POLLIN
> events) and since we're now measuring the wall clock time elapsed since
> a given tail pointer was read the mechanism no longer cares about
> the OA unit's periodic sampling frequency.
>
> The natural place to handle the tail pointer updates was in
> gen7_oa_buffer_is_empty() which is called as part of blocking reads and
> the hrtimer callback used for polling, and so this was renamed to
> oa_buffer_check() considering the added side effect while checking
> whether the buffer contains data.
>
> Signed-off-by: Robert Bragg 
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  59 -
>  drivers/gpu/drm/i915/i915_perf.c | 275 
> ++-
>  2 files changed, 241 insertions(+), 93 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e732d0b3bf65..7b2bdc6ccb26 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2038,7 +2038,7 @@ struct i915_oa_ops {
> size_t *offset);
>
> /**
> -* @oa_buffer_is_empty: Check if OA buffer empty (false positives OK)
> +* @oa_buffer_check: Check for OA buffer data + update tail
>  *
>  * This is either called via fops or the poll check hrtimer (atomic
>  * ctx) without any locks taken.
> @@ -2051,7 +2051,7 @@ struct i915_oa_ops {
>  * here, which will be handled gracefully - likely resulting in an
>  * %EAGAIN error for userspace.
>  */
> -   bool (*oa_buffer_is_empty)(struct drm_i915_private *dev_priv);
> +   bool (*oa_buffer_check)(struct drm_i915_private *dev_priv);
>  };
>
>  struct drm_i915_private {
> @@ -2383,8 +2383,6 @@ struct drm_i915_private {
> int period_exponent;
> int timestamp_frequency;
>
> -   int tail_margin;
> -
> int metrics_set;
>
> const struct i915_oa_reg *mux_regs;
> @@ -2399,6 +2397,59 @@ struct drm_i915_private {
> int format_size;
>
> /**
> +* Locks reads and writes to all head/tail 
> state
> +*
> +* Consider: the head and tail pointer state
> +* needs to be read consistently from a 
> hrtimer
> +* callback (atomic context) and read() fop
> +* (user context) with tail pointer updates
> +* happening in atomic context and head 
> updates
> +* in user context and the (unlikely)
> +* possibility of read() errors needing to
> +* reset all head/tail state.
> +*
> +* Note: Contention or performance aren't
> +* currently a significant concern here
> +* considering the relatively low frequency of
> +* hrtimer callbacks (5ms period) and that
> +* reads 

Re: [Intel-gfx] DP Aux interfaces inquiry

[Ville Syrjälä]

On Tue, Jan 24, 2017 at 07:51:28PM +, mario.limoncie...@dell.com wrote:
> Hi,
> 
> Recently Synaptics collaborated with Dell on a plugin [1] for fwupd that 
> allows flashing Synaptics MST hubs using the DP aux interface to manipulate 
> the DPCD [2].
> Currently the plugin hardcodes the max number of DP aux devices to look for 
> to 3 (as that's what we've seen so far on HW), but we were wondering:
> 
>   1) If there is a way to query the number of devices that the kernel is 
> going to be creating?
>   2) Are there any instances of more than 3 devices in the wild today 
> that anyone is aware of?

These depend on the board you're dealing with, and on the number of
gpus you have in the system (and whether they actually have drivers
loaded for them).

We should also perhaps expose the aux device node for MST devices. At
which point the number of aux nodes could change dynamically when you
plug MST devices in/out.

So I don't think you should be making any assumptions on the
number/order of these device nodes.

> 
> [1] https://github.com/hughsie/fwupd/tree/master/plugins/synapticsmst
> [2] 
> https://github.com/torvalds/linux/commit/e94cb37b34eb8a88fe847438dba55c3f18bf024a
> 
> Thanks,
> 
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/5] drm/i915/perf: no head/tail ref in gen7_oa_read

[Matthew Auld]

On 24 January 2017 at 01:25, Robert Bragg  wrote:
> This avoids redundantly passing an (inout) head and tail pointer to
> gen7_append_oa_reports() from gen7_oa_read which doesn't need to
> reference either itself.
>
> Moving the head/tail reads and writes into gen7_append_oa_reports should
> have no functional effect except to avoid some redundant head pointer
> writes in cases where nothing was copied to userspace.
>
> This is a stepping stone towards updating how the head and tail pointer
> state is managed to improve the workaround for the OA unit's tail
> pointer race. It reduces the number of places we need to read/write the
> head and tail pointers.
>
> Signed-off-by: Robert Bragg 
Reviewed-by: Matthew Auld 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/5] drm/i915/perf: avoid read back of head register

[Matthew Auld]

On 24 January 2017 at 01:25, Robert Bragg  wrote:
> There's no need for the driver to keep reading back the head pointer
> from hardware since the hardware doesn't update it automatically. This
> way we can treat any invalid head pointer value as a software/driver
> bug instead of spurious hardware behaviour.
>
> This change is also a small stepping stone towards re-working how
> the head and tail state is managed as part of an improved workaround
> for the tail register race condition.
>
> Signed-off-by: Robert Bragg 
> ---
>  drivers/gpu/drm/i915/i915_drv.h  | 11 ++
>  drivers/gpu/drm/i915/i915_perf.c | 46 
> ++--
>  2 files changed, 32 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 38509505424d..e732d0b3bf65 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2397,6 +2397,17 @@ struct drm_i915_private {
> u8 *vaddr;
> int format;
> int format_size;
> +
> +   /**
> +* Although we can always read back the head
> +* pointer register, we prefer to avoid
> +* trusting the HW state, just to avoid any
> +* risk that some hardware condition could
> +* somehow bump the head pointer unpredictably
> +* and cause us to forward the wrong OA buffer
> +* data to uesrspace.
"userspace"

> +*/
> +   u32 head;
> } oa_buffer;
>
> u32 gen7_latched_oastatus1;
> diff --git a/drivers/gpu/drm/i915/i915_perf.c 
> b/drivers/gpu/drm/i915/i915_perf.c
> index 4bb7333dac45..e85583d0bcff 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -322,9 +322,8 @@ struct perf_open_properties {
>  static bool gen7_oa_buffer_is_empty_fop_unlocked(struct drm_i915_private 
> *dev_priv)
>  {
> int report_size = dev_priv->perf.oa.oa_buffer.format_size;
> -   u32 oastatus2 = I915_READ(GEN7_OASTATUS2);
> u32 oastatus1 = I915_READ(GEN7_OASTATUS1);
> -   u32 head = oastatus2 & GEN7_OASTATUS2_HEAD_MASK;
> +   u32 head = dev_priv->perf.oa.oa_buffer.head;
> u32 tail = oastatus1 & GEN7_OASTATUS1_TAIL_MASK;
>
> return OA_TAKEN(tail, head) <
> @@ -458,16 +457,24 @@ static int gen7_append_oa_reports(struct 
> i915_perf_stream *stream,
> return -EIO;
>
> head = *head_ptr - gtt_offset;
> +
> +   /* An out of bounds or misaligned head pointer implies a driver bug
> +* since we are in full control of head pointer which should only
> +* be incremented by multiples of the report size (notably also
> +* all a power of two).
> +*/
> +   if (WARN_ONCE(head > OA_BUFFER_SIZE || head % report_size,
> + "Inconsistent OA buffer head pointer = %u\n", head))
> +   return -EIO;
> +
> tail -= gtt_offset;
>
> /* The OA unit is expected to wrap the tail pointer according to the 
> OA
> -* buffer size and since we should never write a misaligned head
> -* pointer we don't expect to read one back either...
> +* buffer size
>  */
> -   if (tail > OA_BUFFER_SIZE || head > OA_BUFFER_SIZE ||
> -   head % report_size) {
> -   DRM_ERROR("Inconsistent OA buffer pointer (head = %u, tail = 
> %u): force restart\n",
> - head, tail);
> +   if (tail > OA_BUFFER_SIZE) {
> +   DRM_ERROR("Inconsistent OA buffer tail pointer = %u: force 
> restart\n",
> + tail);
> dev_priv->perf.oa.ops.oa_disable(dev_priv);
> dev_priv->perf.oa.ops.oa_enable(dev_priv);
> *head_ptr = I915_READ(GEN7_OASTATUS2) &
> @@ -562,8 +569,6 @@ static int gen7_oa_read(struct i915_perf_stream *stream,
> size_t *offset)
>  {
> struct drm_i915_private *dev_priv = stream->dev_priv;
> -   int report_size = dev_priv->perf.oa.oa_buffer.format_size;
> -   u32 oastatus2;
> u32 oastatus1;
> u32 head;
> u32 tail;
> @@ -572,10 +577,9 @@ static int gen7_oa_read(struct i915_perf_stream *stream,
> if (WARN_ON(!dev_priv->perf.oa.oa_buffer.vaddr))
> return -EIO;
>
> -   oastatus2 = I915_READ(GEN7_OASTATUS2);
> oastatus1 = I915_READ(GEN7_OASTATUS1);
>
> -   head = oastatus2 & GEN7_OASTATUS2_HEAD_MASK;
> +   head = dev_priv->perf.oa.oa_buffer.head;
> tail = oastatus1 & GEN7_OASTATUS1_TAIL_MASK;
>
> /* XXX: On Haswell 

Re: [Intel-gfx] [PATCH 2/5] drm/i915/perf: avoid poll, read, EAGAIN busy loops

[Matthew Auld]

On 24 January 2017 at 01:25, Robert Bragg  wrote:
> If the function for checking whether there is OA buffer data available
> (during a poll or blocking read) has false positives then we want to
> avoid a situation where the subsequent read() returns EAGAIN (after
> a more accurate check) followed by a poll() immediately reporting
> the same false positive POLLIN event and effectively maintaining a
> busy loop until there really is data.
>
> This makes sure that we clear the .pollin event status whenever we
> return EAGAIN to userspace which will throttle subsequent POLLIN events
> and repeated attempts to read to the 5ms intervals of the hrtimer
> callback we have.
>
> Signed-off-by: Robert Bragg 
Reviewed-by: Matthew Auld 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/5] drm/i915/perf: fix gen7_append_oa_reports comment

[Matthew Auld]

On 24 January 2017 at 01:25, Robert Bragg  wrote:
> If I'm going to complain about a back-to-front convention then the least
> I can do is not muddle the comment up too.
>
> Signed-off-by: Robert Bragg 
Reviewed-by: Matthew Auld 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: Don't race connector registration

[Daniel Vetter]

On Wed, Jan 25, 2017 at 07:20:45AM -0800, Dave Hansen wrote:
> On 01/24/2017 10:21 PM, Daniel Vetter wrote:
> > Hi Dave,
> > 
> > Still waiting for your testing results on this one here ...
> 
> It's definitely stable with that patch applied.  No more crashes.
> 
> But, it's also definitely having difficulty re-probing to find the
> monitor that's attached to the dock in some cases.  Whatever is going on
> isn't fixed by poking it with xrandr.

Is this new? When exactly does this happen? Does the mst sink connector no
longer show up, or is the connected/disconnected status all wrong?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 15/15] drm: Update kerneldoc for drm_crtc.[hc]

[Daniel Vetter]

On Wed, Jan 25, 2017 at 02:26:09PM +, Eric Engestrom wrote:
> On Wednesday, 2017-01-25 07:26:57 +0100, Daniel Vetter wrote:
> > After going through all the trouble of splitting out parts from
> > drm_crtc.[hc] and then properly documenting each I've entirely
> > forgotten to show the same TLC for CRTCs themselves!
> > 
> > Let's make amends asap.
> > 
> > Signed-off-by: Daniel Vetter 
> > ---
> >  Documentation/gpu/drm-kms.rst |  6 ++
> >  drivers/gpu/drm/drm_crtc.c| 21 +
> >  include/drm/drm_crtc.h| 25 +++--
> >  3 files changed, 46 insertions(+), 6 deletions(-)
> > 
> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > index 35c41cf84a1b..979cee853bb1 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -284,6 +284,12 @@ Atomic Mode Setting Function Reference
> >  CRTC Abstraction
> >  
> >  
> > +.. kernel-doc:: drivers/gpu/drm/drm_crtc.c
> > +   :doc: overview
> > +
> > +CRTC Functions Reference
> > +
> > +
> >  .. kernel-doc:: include/drm/drm_crtc.h
> > :internal:
> >  
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index 5f28e3a5a3e0..6915f897bd8e 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -47,6 +47,27 @@
> >  #include "drm_internal.h"
> >  
> >  /**
> > + * DOC: overview
> > + *
> > + * A CRTC represents the overall display pipeline. It receives pixel data 
> > from
> > + * _plane and blends them together. The _display_mode is also 
> > attached
> > + * to the CRTC, specifying display timings. On the output side the data is 
> > fed
> > + * to one or more _encoder, which are then each connected to one
> > + * _connector.
> > + *
> > + * To create a CRTC, a KMS drivers allocates and zeroes an instances of
> > + *  drm_crtc (possibly as part of a larger structure) and registers 
> > it
> > + * with a call to drm_crtc_init_with_planes().
> > + *
> > + * The CRTC is also the entry point for legacy modeset operations, see
> > + * _crtc_funcs.set_config, legacy plane operations, see
> > + * _crtc_funcs.page_flip and _crtc_funcs.cursor_set2, and other 
> > legacy
> > + * operations like _crtc_funcs.gamma_set. For atomic drivers all these
> > + * features are controlled through _property and
> > + * _mode_config_funcs.atomic_check and 
> > _mode_config_funcs.atomic_check.
> > + */
> > +
> > +/**
> >   * drm_crtc_from_index - find the registered CRTC at an index
> >   * @dev: DRM device
> >   * @idx: index of registered CRTC to find for
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index 816edab054e6..d788c624f67a 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -641,7 +641,7 @@ struct drm_crtc {
> >  *
> >  * This provides a read lock for the overall crtc state (mode, dpms
> >  * state, ...) and a write lock for everything which can be update
> > -* without a full modeset (fb, cursor data, crtc properties ...). Full
> > +* without a full modeset (fb, cursor data, crtc properties ...). A full
> >  * modeset also need to grab _mode_config.connection_mutex.
> >  */
> > struct drm_modeset_lock mutex;
> > @@ -774,10 +774,8 @@ struct drm_crtc {
> >   * @connectors: array of connectors to drive with this CRTC if possible
> >   * @num_connectors: size of @connectors array
> >   *
> > - * Represents a single crtc the connectors that it drives with what mode
> > - * and from which framebuffer it scans out from.
> > - *
> > - * This is used to set modes.
> > + * This represents a modeset configuration for the legacy SETCRTC ioctl 
> > and is
> > + * also used internally. Atomic drivers instead use _atomic_state.
> >   */
> >  struct drm_mode_set {
> > struct drm_framebuffer *fb;
> > @@ -832,7 +830,15 @@ int drm_crtc_force_disable_all(struct drm_device *dev);
> >  int drm_mode_set_config_internal(struct drm_mode_set *set);
> >  struct drm_crtc *drm_crtc_from_index(struct drm_device *dev, int idx);
> >  
> > -/* Helpers */
> > +/**
> > + * drm_crtc_find - look up a CRTC object from its ID
> > + * @dev: DRM device
> > + * @id: _mode_object ID
> > + *
> > + * This can be used to look up a CRTC from its userspace ID. Only used by
> > + * drivers for legacy IOCTLs and interface, nowadays extensions to the KMS
> > + * userspace interface should be done using _property.
> > + */
> >  static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
> > uint32_t id)
> >  {
> > @@ -841,6 +847,13 @@ static inline struct drm_crtc *drm_crtc_find(struct 
> > drm_device *dev,
> > return mo ? obj_to_crtc(mo) : NULL;
> >  }
> >  
> > +/**
> > + * drm_for_each_crtc - iterate over all encoders
> 
> s/encoder/crtc/ :)
> 
> Other than that, this and #3 (btw I know you didn't introduce any of the
> typos in #3, but you can fix them :P) are:
> Reviewed-by: Eric 

Re: [Intel-gfx] [PATCH 06/15] drm/doc: Clarify connector overview

[Daniel Vetter]

On Wed, Jan 25, 2017 at 10:57:17AM -0200, Gustavo Padovan wrote:
> Hi Daniel,
> 
> 2017-01-25 Daniel Vetter :
> 
> > There was a bit of mix-up between initialization and registering.
> > 
> > Signed-off-by: Daniel Vetter 
> > ---
> >  drivers/gpu/drm/drm_connector.c | 9 -
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_connector.c 
> > b/drivers/gpu/drm/drm_connector.c
> > index dd720d4cb4f7..c75ab242f907 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -40,11 +40,10 @@
> >   *
> >   * KMS driver must create, initialize, register and attach at a 
> >   * drm_connector for each such sink. The instance is created as other KMS
> > - * objects and initialized by setting the following fields.
> > - *
> > - * The connector is then registered with a call to drm_connector_init() 
> > with a
> > - * pointer to the connector functions and a connector type, and exposed 
> > through
> > - * sysfs with a call to drm_connector_register().
> > + * objects and initialized by setting the following fields. The connector 
> > is
> > + * initialized with a call to drm_connector_init() with a pointer to the
> > + * connector functions and a connector type, and then exposed to userspace 
> > with
> 
> _connector_funcs

I'm not really clear what you want me to do here ...
-Daniel

> 
> Other than that:
> 
> Reviewed-by: Gustavo Padovan 

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

Re: [Intel-gfx] [PATCH 1/3] [v2] drm: Add new DRM_IOCTL_MODE_GETPLANE2

[Ville Syrjälä]

On Tue, Jan 24, 2017 at 09:20:49PM -0800, Ben Widawsky wrote:
> Originally based off of a patch by Kristian.
> 
> This new ioctl extends DRM_IOCTL_MODE_GETPLANE, by returning information
> about the modifiers that will work with each format.
> 
> It's modified from Kristian's patch in that the modifiers and formats
> are setup by the driver, and then a callback is used to create the
> format list. The LOC was enough difference that I don't think it made
> sense to leave his authorship, but the new UABI was primarily his idea.
> 
> Additionally, I hit a couple of drivers which Kristian missed updating.
> 
> It also contains a change requested by Daniel to make the modifiers
> array a sentinel based structure instead of a sized one. Upon discussion
> on IRC, it was determined that having an invalid modifier might make
> sense in general as well.
> 
> v2:
>   - Make formats uint32_t, and use an offset, see the comment in the
>   patch. Add a WARN_ON and early bail for when there are more than 32
>   formats. (Rob)
>   - Name format_modifiers annotation properly (Ville)
>   - Remove DRM_DEBUG_KMS (Ville)
>   - make flags come before count in struct (Ville)
> 
> Cc: Rob Clark 
> Cc: Ville Syrjälä 
> Cc: Daniel Stone 
> Cc: "Kristian H. Kristensen" 
> References: https://patchwork.kernel.org/patch/9482393/
> Signed-off-by: Ben Widawsky 
> ---
>  drivers/gpu/drm/arc/arcpgu_crtc.c   |  1 +
>  drivers/gpu/drm/arm/hdlcd_crtc.c|  1 +
>  drivers/gpu/drm/arm/malidp_planes.c |  2 +-
>  drivers/gpu/drm/armada/armada_crtc.c|  1 +
>  drivers/gpu/drm/armada/armada_overlay.c |  1 +
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c |  4 +-
>  drivers/gpu/drm/drm_ioctl.c |  2 +-
>  drivers/gpu/drm/drm_modeset_helper.c|  1 +
>  drivers/gpu/drm/drm_plane.c | 65 
> -
>  drivers/gpu/drm/drm_simple_kms_helper.c |  3 ++
>  drivers/gpu/drm/exynos/exynos_drm_plane.c   |  2 +-
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c |  2 +-
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c  |  1 +
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c |  2 +-
>  drivers/gpu/drm/i915/intel_display.c|  7 ++-
>  drivers/gpu/drm/i915/intel_sprite.c |  4 +-
>  drivers/gpu/drm/imx/ipuv3-plane.c   |  4 +-
>  drivers/gpu/drm/mediatek/mtk_drm_plane.c|  2 +-
>  drivers/gpu/drm/meson/meson_plane.c |  1 +
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c   |  2 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   |  2 +-
>  drivers/gpu/drm/mxsfb/mxsfb_drv.c   |  2 +-
>  drivers/gpu/drm/nouveau/nv50_display.c  |  5 +-
>  drivers/gpu/drm/omapdrm/omap_plane.c|  3 +-
>  drivers/gpu/drm/rcar-du/rcar_du_plane.c |  4 +-
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c   |  5 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  4 +-
>  drivers/gpu/drm/sti/sti_cursor.c|  1 +
>  drivers/gpu/drm/sti/sti_gdp.c   |  2 +-
>  drivers/gpu/drm/sti/sti_hqvdp.c |  2 +-
>  drivers/gpu/drm/sun4i/sun4i_layer.c |  1 +
>  drivers/gpu/drm/tegra/dc.c  | 12 ++---
>  drivers/gpu/drm/vc4/vc4_plane.c |  2 +-
>  drivers/gpu/drm/virtio/virtgpu_plane.c  |  2 +-
>  drivers/gpu/drm/zte/zx_plane.c  |  2 +-
>  include/drm/drm_plane.h | 21 +++-
>  include/drm/drm_simple_kms_helper.h |  1 +
>  include/uapi/drm/drm.h  |  1 +
>  include/uapi/drm/drm_fourcc.h   | 11 +
>  include/uapi/drm/drm_mode.h | 36 ++
>  40 files changed, 189 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c 
> b/drivers/gpu/drm/arc/arcpgu_crtc.c
> index ad9a95916f1f..cd8a24c7c67d 100644
> --- a/drivers/gpu/drm/arc/arcpgu_crtc.c
> +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
> @@ -218,6 +218,7 @@ static struct drm_plane *arc_pgu_plane_init(struct 
> drm_device *drm)
>  
>   ret = drm_universal_plane_init(drm, plane, 0xff, _pgu_plane_funcs,
>  formats, ARRAY_SIZE(formats),
> +NULL,
>  DRM_PLANE_TYPE_PRIMARY, NULL);
>   if (ret)
>   return ERR_PTR(ret);
> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c 
> b/drivers/gpu/drm/arm/hdlcd_crtc.c
> index 20ebfb4fbdfa..89fded880807 100644
> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> @@ -283,6 +283,7 @@ static struct drm_plane *hdlcd_plane_init(struct 
> drm_device *drm)
>  
>   ret = drm_universal_plane_init(drm, plane, 0xff, _plane_funcs,
>  formats, 

Re: [Intel-gfx] [PATCH 05/15] drm/core: Use recommened kerneldoc for struct member refs

[Daniel Vetter]

On Wed, Jan 25, 2017 at 10:55:21AM -0200, Gustavo Padovan wrote:
> Otherwise looks good:

Fixed up all and applied

> Reviewed-by: Gustavo Padovan 

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

Re: [Intel-gfx] [PATCH] drm: Don't race connector registration

[Dave Hansen]

On 01/24/2017 10:21 PM, Daniel Vetter wrote:
> Hi Dave,
> 
> Still waiting for your testing results on this one here ...

It's definitely stable with that patch applied.  No more crashes.

But, it's also definitely having difficulty re-probing to find the
monitor that's attached to the dock in some cases.  Whatever is going on
isn't fixed by poking it with xrandr.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/15] drm/kms-helpers: Use recommened kerneldoc for struct member refs

[Daniel Vetter]

On Wed, Jan 25, 2017 at 10:48:53AM -0200, Gustavo Padovan wrote:
> Otherwise looks good to me:

Nice catches, fixed them all and applied it.
> 
> Rewiewed-by: Gustavo Padovan 

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

Re: [Intel-gfx] [PATCH v5 0/5] Add support for Legacy HDMI audio drivers

[Takashi Iwai]

On Wed, 25 Jan 2017 15:45:05 +0100,
Jani Nikula wrote:
> 
> On Wed, 25 Jan 2017, Takashi Iwai  wrote:
> > On Tue, 24 Jan 2017 23:57:48 +0100,
> > Jerome Anand wrote:
> >> 
> >> Legacy (CherryTrail/ Baytrail) HDMI audio drivers added
> >> Legacy hdmi audio-Gfx interaction/ interfacing is updated to use 
> >> irq chip framework
> >> 
> >> This patch series has only been tested on hardware with 
> >> a single HDMI connector/pipe and additional work may be needed for newer 
> >> machines with 2 connectors
> >> 
> >> Jerome Anand (5):
> >>   drm/i915: setup bridge for HDMI LPE audio driver
> >>   drm/i915: Add support for audio driver notifications
> >>   ALSA: add Intel HDMI LPE audio driver for BYT/CHT-T
> >>   ALSA: x86: hdmi: Add audio support for BYT and CHT
> >>   ALSA: x86: hdmi: continue playback even when display  resolution
> >> changes
> >
> > OK, now I merged the patch to topic/intel-lpe-audio branch of my sound
> > git tree.  It's based on 4.10-rc5.
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git 
> > topic/intel-lpe-audio
> >
> > Jani, could you check whether the branch can be merged cleanly to i915
> > tree (just to be sure)?  I'm going to merge this topic branch to
> > for-next branch, maybe tomorrow or on Friday, once after finishing the
> > local build tests.
> 
> There's a trivial merge conflict in Documentation/gpu/i915.rst, I don't
> think that's something to worry about.

OK, thanks for a quick check!


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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Add MIPI_IO WA and program DSI regulators (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Add MIPI_IO WA and program DSI regulators (rev2)
URL   : https://patchwork.freedesktop.org/series/18223/
State : success

== Summary ==

Series 18223v2 drm/i915: Add MIPI_IO WA and program DSI regulators
https://patchwork.freedesktop.org/api/1.0/series/18223/revisions/2/mbox/


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

396d17a6de32b4ef6cf1b531248e25ca6efe8001 drm-tip: 2017y-01m-25d-11h-07m-11s UTC 
integration manifest
26171ea drm/i915: Add MIPI_IO WA and program DSI regulators

== Logs ==

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

Re: [Intel-gfx] [PATCH v2 34/38] drm/i915: Live testing for context execution

[Joonas Lahtinen]

On to, 2017-01-19 at 11:41 +, Chris Wilson wrote:
> Check we can create and execution within a context.
> 
> Signed-off-by: Chris Wilson 



> +static struct i915_vma *
> +gpu_fill_pages(struct i915_vma *vma, u64 offset, unsigned long count, u32 
> value)
> +{

It smells like goto err; in this function.

> +}

Other than that, -EMAGIC, too many numbers and weakly named variables,
can't really follow the test.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v5 0/5] Add support for Legacy HDMI audio drivers

[Jani Nikula]

On Wed, 25 Jan 2017, Takashi Iwai  wrote:
> On Tue, 24 Jan 2017 23:57:48 +0100,
> Jerome Anand wrote:
>> 
>> Legacy (CherryTrail/ Baytrail) HDMI audio drivers added
>> Legacy hdmi audio-Gfx interaction/ interfacing is updated to use 
>> irq chip framework
>> 
>> This patch series has only been tested on hardware with 
>> a single HDMI connector/pipe and additional work may be needed for newer 
>> machines with 2 connectors
>> 
>> Jerome Anand (5):
>>   drm/i915: setup bridge for HDMI LPE audio driver
>>   drm/i915: Add support for audio driver notifications
>>   ALSA: add Intel HDMI LPE audio driver for BYT/CHT-T
>>   ALSA: x86: hdmi: Add audio support for BYT and CHT
>>   ALSA: x86: hdmi: continue playback even when display  resolution
>> changes
>
> OK, now I merged the patch to topic/intel-lpe-audio branch of my sound
> git tree.  It's based on 4.10-rc5.
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git 
> topic/intel-lpe-audio
>
> Jani, could you check whether the branch can be merged cleanly to i915
> tree (just to be sure)?  I'm going to merge this topic branch to
> for-next branch, maybe tomorrow or on Friday, once after finishing the
> local build tests.

There's a trivial merge conflict in Documentation/gpu/i915.rst, I don't
think that's something to worry about.

BR,
Jani.


>
>
> thanks,
>
> Takashi

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

Re: [Intel-gfx] [PATCH 15/15] drm: Update kerneldoc for drm_crtc.[hc]

[Eric Engestrom]

On Wednesday, 2017-01-25 07:26:57 +0100, Daniel Vetter wrote:
> After going through all the trouble of splitting out parts from
> drm_crtc.[hc] and then properly documenting each I've entirely
> forgotten to show the same TLC for CRTCs themselves!
> 
> Let's make amends asap.
> 
> Signed-off-by: Daniel Vetter 
> ---
>  Documentation/gpu/drm-kms.rst |  6 ++
>  drivers/gpu/drm/drm_crtc.c| 21 +
>  include/drm/drm_crtc.h| 25 +++--
>  3 files changed, 46 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 35c41cf84a1b..979cee853bb1 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -284,6 +284,12 @@ Atomic Mode Setting Function Reference
>  CRTC Abstraction
>  
>  
> +.. kernel-doc:: drivers/gpu/drm/drm_crtc.c
> +   :doc: overview
> +
> +CRTC Functions Reference
> +
> +
>  .. kernel-doc:: include/drm/drm_crtc.h
> :internal:
>  
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 5f28e3a5a3e0..6915f897bd8e 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -47,6 +47,27 @@
>  #include "drm_internal.h"
>  
>  /**
> + * DOC: overview
> + *
> + * A CRTC represents the overall display pipeline. It receives pixel data 
> from
> + * _plane and blends them together. The _display_mode is also 
> attached
> + * to the CRTC, specifying display timings. On the output side the data is 
> fed
> + * to one or more _encoder, which are then each connected to one
> + * _connector.
> + *
> + * To create a CRTC, a KMS drivers allocates and zeroes an instances of
> + *  drm_crtc (possibly as part of a larger structure) and registers it
> + * with a call to drm_crtc_init_with_planes().
> + *
> + * The CRTC is also the entry point for legacy modeset operations, see
> + * _crtc_funcs.set_config, legacy plane operations, see
> + * _crtc_funcs.page_flip and _crtc_funcs.cursor_set2, and other 
> legacy
> + * operations like _crtc_funcs.gamma_set. For atomic drivers all these
> + * features are controlled through _property and
> + * _mode_config_funcs.atomic_check and 
> _mode_config_funcs.atomic_check.
> + */
> +
> +/**
>   * drm_crtc_from_index - find the registered CRTC at an index
>   * @dev: DRM device
>   * @idx: index of registered CRTC to find for
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 816edab054e6..d788c624f67a 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -641,7 +641,7 @@ struct drm_crtc {
>*
>* This provides a read lock for the overall crtc state (mode, dpms
>* state, ...) and a write lock for everything which can be update
> -  * without a full modeset (fb, cursor data, crtc properties ...). Full
> +  * without a full modeset (fb, cursor data, crtc properties ...). A full
>* modeset also need to grab _mode_config.connection_mutex.
>*/
>   struct drm_modeset_lock mutex;
> @@ -774,10 +774,8 @@ struct drm_crtc {
>   * @connectors: array of connectors to drive with this CRTC if possible
>   * @num_connectors: size of @connectors array
>   *
> - * Represents a single crtc the connectors that it drives with what mode
> - * and from which framebuffer it scans out from.
> - *
> - * This is used to set modes.
> + * This represents a modeset configuration for the legacy SETCRTC ioctl and 
> is
> + * also used internally. Atomic drivers instead use _atomic_state.
>   */
>  struct drm_mode_set {
>   struct drm_framebuffer *fb;
> @@ -832,7 +830,15 @@ int drm_crtc_force_disable_all(struct drm_device *dev);
>  int drm_mode_set_config_internal(struct drm_mode_set *set);
>  struct drm_crtc *drm_crtc_from_index(struct drm_device *dev, int idx);
>  
> -/* Helpers */
> +/**
> + * drm_crtc_find - look up a CRTC object from its ID
> + * @dev: DRM device
> + * @id: _mode_object ID
> + *
> + * This can be used to look up a CRTC from its userspace ID. Only used by
> + * drivers for legacy IOCTLs and interface, nowadays extensions to the KMS
> + * userspace interface should be done using _property.
> + */
>  static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
>   uint32_t id)
>  {
> @@ -841,6 +847,13 @@ static inline struct drm_crtc *drm_crtc_find(struct 
> drm_device *dev,
>   return mo ? obj_to_crtc(mo) : NULL;
>  }
>  
> +/**
> + * drm_for_each_crtc - iterate over all encoders

s/encoder/crtc/ :)

Other than that, this and #3 (btw I know you didn't introduce any of the
typos in #3, but you can fix them :P) are:
Reviewed-by: Eric Engestrom 

> + * @crtc: a  drm_crtc as the loop cursor
> + * @dev: the  drm_device
> + *
> + * Iterate over all CRTCs of @dev.
> + */
>  #define drm_for_each_crtc(crtc, dev) \
>   list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head)
>  
> -- 
> 2.11.0
> 

[Intel-gfx] ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Report the failure to write to the punit

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Report the failure to write to the 
punit
URL   : https://patchwork.freedesktop.org/series/18549/
State : warning

== Summary ==

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

Test kms_force_connector_basic:
Subgroup force-load-detect:
pass   -> DMESG-WARN (fi-snb-2520m)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-a:
pass   -> DMESG-WARN (fi-ivb-3770)

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

396d17a6de32b4ef6cf1b531248e25ca6efe8001 drm-tip: 2017y-01m-25d-11h-07m-11s UTC 
integration manifest
909089a drm/i915: Also clear the punit's PDATA sideband register
f86bb68 drm/i915: Report the failure to write to the punit

== Logs ==

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

[Intel-gfx] [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators

[Vidya Srinivas]

From: Uma Shankar 

Enable MIPI IO WA for BXT DSI as per bspec and
program the DSI regulators.

v2: Moved IO enable to pre-enable as per Mika's
review comments. Also reused the existing register
definition for BXT_P_CR_GT_DISP_PWRON.

v3: Added Programming the DSI regulators as per disable/enable
sequences.

v4: Restricting regulator changes to BXT as suggested by
Jani/Mika

v5: Removed redundant read/modify for regulator register as
per Jani's comment. Maintain enable/disable symmetry as per spec.

Signed-off-by: Uma Shankar 
Signed-off-by: Vidya Srinivas 
---
 drivers/gpu/drm/i915/i915_reg.h  |  7 +++
 drivers/gpu/drm/i915/intel_dsi.c | 24 
 2 files changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 00970aa..0a9ad44 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1553,6 +1553,7 @@ enum skl_disp_power_wells {
_MMIO(_BXT_PHY_CH(phy, ch, reg_ch0, reg_ch1))
 
 #define BXT_P_CR_GT_DISP_PWRON _MMIO(0x138090)
+#define  MIPIO_RST_CTRL(1 << 2)
 
 #define _BXT_PHY_CTL_DDI_A 0x64C00
 #define _BXT_PHY_CTL_DDI_B 0x64C10
@@ -8301,6 +8302,12 @@ enum {
 #define _BXT_MIPIC_PORT_CTRL   0x6B8C0
 #define BXT_MIPI_PORT_CTRL(tc) _MMIO_MIPI(tc, _BXT_MIPIA_PORT_CTRL, 
_BXT_MIPIC_PORT_CTRL)
 
+#define BXT_P_DSI_REGULATOR_CFG_MMIO(0x160020)
+#define  STAP_SELECT   (1 << 0)
+
+#define BXT_P_DSI_REGULATOR_TX_CTRL_MMIO(0x160054)
+#define  HS_IO_CTRL_SELECT (1 << 0)
+
 #define  DPI_ENABLE(1 << 31) /* A + C */
 #define  MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27
 #define  MIPIA_MIPI4DPHY_DELAY_COUNT_MASK  (0xf << 27)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 16732e7..c98234e 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -548,6 +548,7 @@ static void intel_dsi_pre_enable(struct intel_encoder 
*encoder,
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
struct intel_dsi *intel_dsi = enc_to_intel_dsi(>base);
enum port port;
+   u32 val;
 
DRM_DEBUG_KMS("\n");
 
@@ -558,6 +559,17 @@ static void intel_dsi_pre_enable(struct intel_encoder 
*encoder,
intel_disable_dsi_pll(encoder);
intel_enable_dsi_pll(encoder, pipe_config);
 
+   if (IS_BROXTON(dev_priv)) {
+   /* Add MIPI IO reset programming for modeset */
+   val = I915_READ(BXT_P_CR_GT_DISP_PWRON);
+   I915_WRITE(BXT_P_CR_GT_DISP_PWRON,
+   val | MIPIO_RST_CTRL);
+
+   /* Power up DSI regulator */
+   I915_WRITE(BXT_P_DSI_REGULATOR_CFG, STAP_SELECT);
+   I915_WRITE(BXT_P_DSI_REGULATOR_TX_CTRL, 0);
+   }
+
intel_dsi_prepare(encoder, pipe_config);
 
/* Panel Enable over CRC PMIC */
@@ -707,6 +719,7 @@ static void intel_dsi_post_disable(struct intel_encoder 
*encoder,
 {
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
struct intel_dsi *intel_dsi = enc_to_intel_dsi(>base);
+   u32 val;
 
DRM_DEBUG_KMS("\n");
 
@@ -714,6 +727,17 @@ static void intel_dsi_post_disable(struct intel_encoder 
*encoder,
 
intel_dsi_clear_device_ready(encoder);
 
+   if (IS_BROXTON(dev_priv)) {
+   /* Power down DSI regulator to save power */
+   I915_WRITE(BXT_P_DSI_REGULATOR_CFG, STAP_SELECT);
+   I915_WRITE(BXT_P_DSI_REGULATOR_TX_CTRL, HS_IO_CTRL_SELECT);
+
+   /* Add MIPI IO reset programming for modeset */
+   val = I915_READ(BXT_P_CR_GT_DISP_PWRON);
+   I915_WRITE(BXT_P_CR_GT_DISP_PWRON,
+   val & ~MIPIO_RST_CTRL);
+   }
+
intel_disable_dsi_pll(encoder);
 
if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH v2 3/3] drm/i915: clean up unused vgpu_read/write

[Chris Wilson]

On Wed, Jan 25, 2017 at 09:44:52PM +0800, Weinan Li wrote:

Having converted the force_wake_get/_put routines for a vGPU to be
no-op, we can use the common mmio accessors and remove our specialised
routines that simply skipped the calls to control force_wake.

> Signed-off-by: Weinan Li 
Reviewed-by: Chris Wilson 
-Chris

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

Re: [Intel-gfx] [PATCH v2 2/3] drm/i915: noop forcewake get/put when vgpu activated

[Chris Wilson]

On Wed, Jan 25, 2017 at 09:44:51PM +0800, Weinan Li wrote:
> Host maintian the hardware's forcewake state, guest don't need and also
> can't control it. Although vgpu_read/write bypass forcewake_get/put in MMIO
> read/write, but still have separate path called by
> "intel_uncore_forcewake_get/put" and
> "intel_uncore_forcewake_get/put__locked". Unnecessary MMIO access in guest
> waste much CPU cost. Since we full virtualize the MMIO, just noop the
> forcewake get/put.

For a virtualised GPU, the host maintains the forcewake state on the
real device. As we don't control forcewake ourselves, we can simply
set force_wake_get() and force_wake_put() to be no-ops. By setting the
vfuncs, we adjust both the manual control of forcewake and around the
mmio accessors (making our vgpu specific mmio routines redundant and
to be removed in the next patch).

> Signed-off-by: Weinan Li 
Reviewed-by: Chris Wilson 
-Chris

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

Re: [Intel-gfx] [PATCH 04/14] drm/i915: Clean up the .get_cdclk() assignment if ladder

[Ville Syrjälä]

On Tue, Jan 24, 2017 at 02:29:00PM +0200, David Weinehall wrote:
> On Fri, Jan 20, 2017 at 08:21:55PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> > 
> > Let's clean up the mess we have in the if ladder that assigns the
> > .get_cdclk() hooks. The grouping of the platforms by the function
> > results in a thing that's not really legible, so let's do it the
> > other way around and order the if ladder by platform and duplicate
> > whatever assignments we need.
> > 
> > To further avoid confusion with the function names let's rename
> > them to just fixed__get_cdclk(). The other option would
> > be to duplicate the functions entirely but it seems quite
> > pointless to do that since each one just returns a fixed value.
> > 
> > Signed-off-by: Ville Syrjälä 
> > Reviewed-by: Ander Conselvan de Oliveira 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 41 
> > +---
> >  1 file changed, 24 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index bd0fdc04bc92..668c61ad45a6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -7366,22 +7366,22 @@ static int valleyview_get_cdclk(struct 
> > drm_i915_private *dev_priv)
> >   CCK_DISPLAY_CLOCK_CONTROL);
> >  }
> >  
> > -static int ilk_get_cdclk(struct drm_i915_private *dev_priv)
> > +static int fixed_450mhz_get_cdclk(struct drm_i915_private *dev_priv)
> >  {
> > return 45;
> >  }
> >  
> > -static int i945_get_cdclk(struct drm_i915_private *dev_priv)
> > +static int fixed_400mhz_get_cdclk(struct drm_i915_private *dev_priv)
> >  {
> > return 40;
> >  }
> >  
> > -static int i915_get_cdclk(struct drm_i915_private *dev_priv)
> > +static int fixed_333mhz_get_cdclk(struct drm_i915_private *dev_priv)
> >  {
> > return 33;
> >  }
> >  
> > -static int i9xx_misc_get_cdclk(struct drm_i915_private *dev_priv)
> > +static int fixed_200mhz_get_cdclk(struct drm_i915_private *dev_priv)
> >  {
> > return 20;
> >  }
> > @@ -7431,7 +7431,7 @@ static int i915gm_get_cdclk(struct drm_i915_private 
> > *dev_priv)
> > }
> >  }
> >  
> > -static int i865_get_cdclk(struct drm_i915_private *dev_priv)
> > +static int fixed_266mhz_get_cdclk(struct drm_i915_private *dev_priv)
> >  {
> > return 27;
> >  }
> 
> How about reordering this and the one above to have the clocks in
> strictly descending order?

It gets reordered in the patch that moves it all into a new file.

> 
> > @@ -7474,7 +7474,7 @@ static int i85x_get_cdclk(struct drm_i915_private 
> > *dev_priv)
> > return 0;
> >  }
> 
> Similarly, move this one down.
> 
> > -static int i830_get_cdclk(struct drm_i915_private *dev_priv)
> > +static int fixed_133mhz_get_cdclk(struct drm_i915_private *dev_priv)
> >  {
> > return 13;
> >  }
> > @@ -16180,32 +16180,39 @@ void intel_init_display_hooks(struct 
> > drm_i915_private *dev_priv)
> > dev_priv->display.get_cdclk = haswell_get_cdclk;
> > else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > dev_priv->display.get_cdclk = valleyview_get_cdclk;
> > +   else if (IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv))
> > +   dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk;
> > else if (IS_GEN5(dev_priv))
> > -   dev_priv->display.get_cdclk = ilk_get_cdclk;
> > -   else if (IS_I945G(dev_priv) || IS_I965G(dev_priv) ||
> > -IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv))
> > -   dev_priv->display.get_cdclk = i945_get_cdclk;
> > +   dev_priv->display.get_cdclk = fixed_450mhz_get_cdclk;
> > else if (IS_GM45(dev_priv))
> > dev_priv->display.get_cdclk = gm45_get_cdclk;
> > +   else if (IS_G4X(dev_priv))
> > +   dev_priv->display.get_cdclk = g33_get_cdclk;
> > else if (IS_I965GM(dev_priv))
> > dev_priv->display.get_cdclk = i965gm_get_cdclk;
> > +   else if (IS_I965G(dev_priv))
> > +   dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk;
> > else if (IS_PINEVIEW(dev_priv))
> > dev_priv->display.get_cdclk = pnv_get_cdclk;
> > -   else if (IS_G33(dev_priv) || IS_G4X(dev_priv))
> > +   else if (IS_G33(dev_priv))
> > dev_priv->display.get_cdclk = g33_get_cdclk;
> > -   else if (IS_I915G(dev_priv))
> > -   dev_priv->display.get_cdclk = i915_get_cdclk;
> > -   else if (IS_I945GM(dev_priv) || IS_I845G(dev_priv))
> > -   dev_priv->display.get_cdclk = i9xx_misc_get_cdclk;
> > +   else if (IS_I945GM(dev_priv))
> > +   dev_priv->display.get_cdclk = fixed_200mhz_get_cdclk;
> > +   else if (IS_I945G(dev_priv))
> > +   dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk;
> > else if (IS_I915GM(dev_priv))
> > dev_priv->display.get_cdclk = 

Re: [Intel-gfx] [PATCH 03/15] drm/kms-core: Use recommened kerneldoc for struct member refs

[Eric Engestrom]

On Wednesday, 2017-01-25 07:26:45 +0100, Daniel Vetter wrote:
> I just learned that _name.member_name works and looks pretty
> even. It doesn't (yet) link to the member directly though, which would
> be really good for big structures or vfunc tables (where the
> per-member kerneldoc tends to be long).
> 
> Also some minor drive-by polish where it makes sense, I read a lot
> of docs ...
> 
> Cc: Jani Nikula 
> Cc: Chris Wilson 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_atomic.c| 71 
> ++---
>  drivers/gpu/drm/drm_blend.c | 11 +++---
>  drivers/gpu/drm/drm_connector.c | 12 +++
>  drivers/gpu/drm/drm_crtc.c  |  7 ++--
>  drivers/gpu/drm/drm_dumb_buffers.c  |  4 +--
>  drivers/gpu/drm/drm_encoder.c   |  2 +-
>  drivers/gpu/drm/drm_encoder_slave.c |  2 +-
>  drivers/gpu/drm/drm_framebuffer.c   | 10 +++---
>  drivers/gpu/drm/drm_modeset_lock.c  | 10 +++---
>  drivers/gpu/drm/drm_plane.c |  2 +-
>  drivers/gpu/drm/drm_property.c  |  2 +-
>  include/drm/drm_atomic.h|  6 ++--
>  include/drm/drm_color_mgmt.h|  2 +-
>  include/drm/drm_connector.h | 40 ++---
>  include/drm/drm_crtc.h  | 29 +++
>  include/drm/drm_framebuffer.h   | 15 
>  include/drm/drm_mode_config.h   | 12 +++
>  include/drm/drm_mode_object.h   | 13 ---
>  include/drm/drm_modeset_lock.h  |  2 +-
>  include/drm/drm_plane.h | 18 +-
>  include/drm/drm_property.h  |  8 ++---
>  21 files changed, 141 insertions(+), 137 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 723392fc98c8..96c81a61a542 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -195,8 +195,8 @@ EXPORT_SYMBOL(drm_atomic_state_default_clear);
>   * all locks. So someone else could sneak in and change the current modeset
>   * configuration. Which means that all the state assembled in @state is no
>   * longer an atomic update to the current state, but to some arbitrary 
> earlier
> - * state. Which could break assumptions the driver's ->atomic_check likely
> - * relies on.
> + * state. Which could break assumptions the driver's
> + * _mode_config_funcs.atomic_check likely relies on.
>   *
>   * Hence we must clear all cached state and completely start over, using this
>   * function.
> @@ -456,11 +456,10 @@ drm_atomic_replace_property_blob_from_id(struct 
> drm_crtc *crtc,
>   * @property: the property to set
>   * @val: the new property value
>   *
> - * Use this instead of calling crtc->atomic_set_property directly.
> - * This function handles generic/core properties and calls out to
> - * driver's ->atomic_set_property() for driver properties.  To ensure
> - * consistent behavior you must call this function rather than the
> - * driver hook directly.
> + * This function handles generic/core properties and calls out to driver's
> + * _crtc_funcs.atomic_set_property for driver properties. To ensure
> + * consistent behavior you must call this function rather than the driver 
> hook
> + * directly.
>   *
>   * RETURNS:
>   * Zero on success, error code on failure
> @@ -532,10 +531,10 @@ EXPORT_SYMBOL(drm_atomic_crtc_set_property);
>   * @property: the property to set
>   * @val: return location for the property value
>   *
> - * This function handles generic/core properties and calls out to
> - * driver's ->atomic_get_property() for driver properties.  To ensure
> - * consistent behavior you must call this function rather than the
> - * driver hook directly.
> + * This function handles generic/core properties and calls out to driver's
> + * _crtc_funcs.atomic_get_property for driver properties. To ensure
> + * consistent behavior you must call this function rather than the driver 
> hook
> + * directly.
>   *
>   * RETURNS:
>   * Zero on success, error code on failure
> @@ -716,11 +715,10 @@ EXPORT_SYMBOL(drm_atomic_get_plane_state);
>   * @property: the property to set
>   * @val: the new property value
>   *
> - * Use this instead of calling plane->atomic_set_property directly.
> - * This function handles generic/core properties and calls out to
> - * driver's ->atomic_set_property() for driver properties.  To ensure
> - * consistent behavior you must call this function rather than the
> - * driver hook directly.
> + * This function handles generic/core properties and calls out to driver's
> + * _plane_funcs.atomic_set_property for driver properties.  To ensure
> + * consistent behavior you must call this function rather than the driver 
> hook
> + * directly.
>   *
>   * RETURNS:
>   * Zero on success, error code on failure
> @@ -791,10 +789,10 @@ EXPORT_SYMBOL(drm_atomic_plane_set_property);
>   * @property: the property to set
>   * @val: return location for the property value
>   *
> - * This function 

Re: [Intel-gfx] [PATCH v5 0/5] Add support for Legacy HDMI audio drivers

[Takashi Iwai]

On Tue, 24 Jan 2017 23:57:48 +0100,
Jerome Anand wrote:
> 
> Legacy (CherryTrail/ Baytrail) HDMI audio drivers added
> Legacy hdmi audio-Gfx interaction/ interfacing is updated to use 
> irq chip framework
> 
> This patch series has only been tested on hardware with 
> a single HDMI connector/pipe and additional work may be needed for newer 
> machines with 2 connectors
> 
> Jerome Anand (5):
>   drm/i915: setup bridge for HDMI LPE audio driver
>   drm/i915: Add support for audio driver notifications
>   ALSA: add Intel HDMI LPE audio driver for BYT/CHT-T
>   ALSA: x86: hdmi: Add audio support for BYT and CHT
>   ALSA: x86: hdmi: continue playback even when display  resolution
> changes

OK, now I merged the patch to topic/intel-lpe-audio branch of my sound
git tree.  It's based on 4.10-rc5.

  git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git 
topic/intel-lpe-audio

Jani, could you check whether the branch can be merged cleanly to i915
tree (just to be sure)?  I'm going to merge this topic branch to
for-next branch, maybe tomorrow or on Friday, once after finishing the
local build tests.


thanks,

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

[Intel-gfx] [PATCH v2 3/3] drm/i915: clean up unused vgpu_read/write

[Weinan Li]

Signed-off-by: Weinan Li 
---
 drivers/gpu/drm/i915/intel_uncore.c | 58 -
 1 file changed, 58 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
b/drivers/gpu/drm/i915/intel_uncore.c
index 9fad4de..e9046fa 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1052,34 +1052,6 @@ static inline void __force_wake_auto(struct 
drm_i915_private *dev_priv,
 #undef GEN6_READ_FOOTER
 #undef GEN6_READ_HEADER
 
-#define VGPU_READ_HEADER(x) \
-   unsigned long irqflags; \
-   u##x val = 0; \
-   assert_rpm_device_not_suspended(dev_priv); \
-   spin_lock_irqsave(_priv->uncore.lock, irqflags)
-
-#define VGPU_READ_FOOTER \
-   spin_unlock_irqrestore(_priv->uncore.lock, irqflags); \
-   trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \
-   return val
-
-#define __vgpu_read(x) \
-static u##x \
-vgpu_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
-   VGPU_READ_HEADER(x); \
-   val = __raw_i915_read##x(dev_priv, reg); \
-   VGPU_READ_FOOTER; \
-}
-
-__vgpu_read(8)
-__vgpu_read(16)
-__vgpu_read(32)
-__vgpu_read(64)
-
-#undef __vgpu_read
-#undef VGPU_READ_FOOTER
-#undef VGPU_READ_HEADER
-
 #define GEN2_WRITE_HEADER \
trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
assert_rpm_wakelock_held(dev_priv); \
@@ -1202,31 +1174,6 @@ static inline void __force_wake_auto(struct 
drm_i915_private *dev_priv,
 #undef GEN6_WRITE_FOOTER
 #undef GEN6_WRITE_HEADER
 
-#define VGPU_WRITE_HEADER \
-   unsigned long irqflags; \
-   trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
-   assert_rpm_device_not_suspended(dev_priv); \
-   spin_lock_irqsave(_priv->uncore.lock, irqflags)
-
-#define VGPU_WRITE_FOOTER \
-   spin_unlock_irqrestore(_priv->uncore.lock, irqflags)
-
-#define __vgpu_write(x) \
-static void vgpu_write##x(struct drm_i915_private *dev_priv, \
- i915_reg_t reg, u##x val, bool trace) { \
-   VGPU_WRITE_HEADER; \
-   __raw_i915_write##x(dev_priv, reg, val); \
-   VGPU_WRITE_FOOTER; \
-}
-
-__vgpu_write(8)
-__vgpu_write(16)
-__vgpu_write(32)
-
-#undef __vgpu_write
-#undef VGPU_WRITE_FOOTER
-#undef VGPU_WRITE_HEADER
-
 #define ASSIGN_WRITE_MMIO_VFUNCS(x) \
 do { \
dev_priv->uncore.funcs.mmio_writeb = x##_write8; \
@@ -1462,11 +1409,6 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
if (INTEL_GEN(dev_priv) >= 8)
intel_shadow_table_check();
 
-   if (intel_vgpu_active(dev_priv)) {
-   ASSIGN_WRITE_MMIO_VFUNCS(vgpu);
-   ASSIGN_READ_MMIO_VFUNCS(vgpu);
-   }
-
i915_check_and_clear_faults(dev_priv);
 }
 #undef ASSIGN_WRITE_MMIO_VFUNCS
-- 
1.9.1

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

[Intel-gfx] [PATCH v2 2/3] drm/i915: noop forcewake get/put when vgpu activated

[Weinan Li]

Host maintian the hardware's forcewake state, guest don't need and also
can't control it. Although vgpu_read/write bypass forcewake_get/put in MMIO
read/write, but still have separate path called by
"intel_uncore_forcewake_get/put" and
"intel_uncore_forcewake_get/put__locked". Unnecessary MMIO access in guest
waste much CPU cost. Since we full virtualize the MMIO, just noop the
forcewake get/put.

Signed-off-by: Weinan Li 
---
 drivers/gpu/drm/i915/intel_uncore.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
b/drivers/gpu/drm/i915/intel_uncore.c
index abe0888..9fad4de 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -133,6 +133,13 @@
 }
 
 static void
+vgpu_fw_domains_nop(struct drm_i915_private *dev_priv,
+   enum forcewake_domains fw_domains)
+{
+   /* Guest driver doesn't need to takes care forcewake. */
+}
+
+static void
 fw_domains_posting_read(struct drm_i915_private *dev_priv)
 {
struct intel_uncore_forcewake_domain *d;
@@ -1374,6 +1381,12 @@ static void intel_uncore_fw_domains_init(struct 
drm_i915_private *dev_priv)
fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
   FORCEWAKE, FORCEWAKE_ACK);
}
+   if (intel_vgpu_active(dev_priv)) {
+   dev_priv->uncore.funcs.force_wake_get =
+   vgpu_fw_domains_nop;
+   dev_priv->uncore.funcs.force_wake_put =
+   vgpu_fw_domains_nop;
+   }
 
/* All future platforms are expected to require complex power gating */
WARN_ON(dev_priv->uncore.fw_domains == 0);
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators

[Shankar, Uma]

>-Original Message-
>From: Nikula, Jani
>Sent: Thursday, January 19, 2017 4:34 PM
>To: Srinivas, Vidya ; intel-
>g...@lists.freedesktop.org
>Cc: Shankar, Uma ; Syrjala, Ville
>
>Subject: Re: [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators
>
>On Thu, 19 Jan 2017, Vidya Srinivas  wrote:
>> From: Uma Shankar 
>>
>> Enable MIPI IO WA for BXT DSI as per bspec and program the DSI
>> regulators.
>>
>> v2: Moved IO enable to pre-enable as per Mika's review comments. Also
>> reused the existing register definition for BXT_P_CR_GT_DISP_PWRON.
>>
>> v3: Added Programming the DSI regulators as per disable/enable
>> sequences.
>>
>> v4: Restricting regulator changes to BXT as suggested by Jani/Mika
>
>This applies to BXT_P_CR_GT_DISP_PWRON changes as well.
Yes, this should be under IS_BXT.

>
>One other question inline.
>
>>
>> Signed-off-by: Uma Shankar 
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h  |  7 +++
>> drivers/gpu/drm/i915/intel_dsi.c | 25 +
>>  2 files changed, 32 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h index 00970aa..0a9ad44 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -1553,6 +1553,7 @@ enum skl_disp_power_wells {
>>  _MMIO(_BXT_PHY_CH(phy, ch, reg_ch0, reg_ch1))
>>
>>  #define BXT_P_CR_GT_DISP_PWRON  _MMIO(0x138090)
>> +#define  MIPIO_RST_CTRL (1 << 2)
>>
>>  #define _BXT_PHY_CTL_DDI_A  0x64C00
>>  #define _BXT_PHY_CTL_DDI_B  0x64C10
>> @@ -8301,6 +8302,12 @@ enum {
>>  #define _BXT_MIPIC_PORT_CTRL0x6B8C0
>>  #define BXT_MIPI_PORT_CTRL(tc)  _MMIO_MIPI(tc,
>_BXT_MIPIA_PORT_CTRL, _BXT_MIPIC_PORT_CTRL)
>>
>> +#define BXT_P_DSI_REGULATOR_CFG
>   _MMIO(0x160020)
>> +#define  STAP_SELECT(1 << 0)
>> +
>> +#define BXT_P_DSI_REGULATOR_TX_CTRL _MMIO(0x160054)
>> +#define  HS_IO_CTRL_SELECT  (1 << 0)
>> +
>>  #define  DPI_ENABLE (1 << 31) /* A
>+ C */
>>  #define  MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT  27
>>  #define  MIPIA_MIPI4DPHY_DELAY_COUNT_MASK   (0xf << 27)
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
>> b/drivers/gpu/drm/i915/intel_dsi.c
>> index 16732e7..4dc1293 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -548,6 +548,7 @@ static void intel_dsi_pre_enable(struct intel_encoder
>*encoder,
>>  struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>  struct intel_dsi *intel_dsi = enc_to_intel_dsi(>base);
>>  enum port port;
>> +u32 val;
>>
>>  DRM_DEBUG_KMS("\n");
>>
>> @@ -558,6 +559,11 @@ static void intel_dsi_pre_enable(struct
>intel_encoder *encoder,
>>  intel_disable_dsi_pll(encoder);
>>  intel_enable_dsi_pll(encoder, pipe_config);
>>
>> +/* Add MIPI IO reset programming for modeset */
>> +val = I915_READ(BXT_P_CR_GT_DISP_PWRON);
>> +I915_WRITE(BXT_P_CR_GT_DISP_PWRON,
>> +val | MIPIO_RST_CTRL);
>> +
>>  intel_dsi_prepare(encoder, pipe_config);
>>
>>  /* Panel Enable over CRC PMIC */
>> @@ -575,6 +581,14 @@ static void intel_dsi_pre_enable(struct
>intel_encoder *encoder,
>>  I915_WRITE(DSPCLK_GATE_D, val);
>>  }
>>
>> +/* Power up DSI regulator */
>> +if (IS_BROXTON(dev_priv)) {
>> +I915_WRITE(BXT_P_DSI_REGULATOR_CFG, STAP_SELECT);
>> +val = I915_READ(BXT_P_DSI_REGULATOR_TX_CTRL);
>> +val &= ~HS_IO_CTRL_SELECT;
>> +I915_WRITE(BXT_P_DSI_REGULATOR_TX_CTRL, val);
>
>Why does this specific change warrant a read-modify-write when the other
>regulator changes in this patch do a full register write?
Checked this in bspec and looks like we can avoid a read/modify operation.

>
>Also, the enable and disable sequences seem a bit asymmetric with these
>changes, i.e. you enable and disable things in different steps of the
>sequences. That's a bit surprising.
Yes, will update this to maintain the symmetry and re-send.
Thanks Jani for all your valuable inputs.

>
>(These might have an answer in bspec, but I don't seem to be able to access
>that right now.)
>
>BR,
>Jani.
>
>> +}
>> +
>>  /* put device in ready state */
>>  intel_dsi_device_ready(encoder);
>>
>> @@ -707,6 +721,7 @@ static void intel_dsi_post_disable(struct
>> intel_encoder *encoder,  {
>>  struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>  struct intel_dsi *intel_dsi = enc_to_intel_dsi(>base);
>> +u32 val;
>>
>>  DRM_DEBUG_KMS("\n");
>>
>> @@ -714,8 +729,18 @@ static void intel_dsi_post_disable(struct
>> intel_encoder *encoder,
>>
>>  intel_dsi_clear_device_ready(encoder);
>>
>> +

[Intel-gfx] [PATCH 2/2] drm/i915: Also clear the punit's PDATA sideband register

[Chris Wilson]

As the previous punit i/o may have failed, the contents of the PDATA are
undefined. Always clear it to 0 prior to sending the command.

Signed-off-by: Chris Wilson 
Cc: Mika Kuoppala 
---
 drivers/gpu/drm/i915/intel_sideband.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sideband.c 
b/drivers/gpu/drm/i915/intel_sideband.c
index d3d49a09c919..9f782b5eb6e6 100644
--- a/drivers/gpu/drm/i915/intel_sideband.c
+++ b/drivers/gpu/drm/i915/intel_sideband.c
@@ -60,8 +60,7 @@ static int vlv_sideband_rw(struct drm_i915_private *dev_priv, 
u32 devfn,
}
 
I915_WRITE(VLV_IOSF_ADDR, addr);
-   if (!is_read)
-   I915_WRITE(VLV_IOSF_DATA, *val);
+   I915_WRITE(VLV_IOSF_DATA, is_read ? 0 : *val);
I915_WRITE(VLV_IOSF_DOORBELL_REQ, cmd);
 
if (intel_wait_for_register(dev_priv,
@@ -74,7 +73,6 @@ static int vlv_sideband_rw(struct drm_i915_private *dev_priv, 
u32 devfn,
 
if (is_read)
*val = I915_READ(VLV_IOSF_DATA);
-   I915_WRITE(VLV_IOSF_DATA, 0);
 
return 0;
 }
-- 
2.11.0

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

[Intel-gfx] [PATCH 1/2] drm/i915: Report the failure to write to the punit

[Chris Wilson]

The write to the punit may fail, so propagate the error code back to its
callers. Of particular interest are the RPS writes, so add appropriate
user error codes and logging.

Signed-off-by: Chris Wilson 
Cc: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_debugfs.c   |  6 --
 drivers/gpu/drm/i915/i915_drv.h   |  4 ++--
 drivers/gpu/drm/i915/i915_sysfs.c |  9 -
 drivers/gpu/drm/i915/intel_pm.c   | 38 ++-
 drivers/gpu/drm/i915/intel_sideband.c | 10 ++---
 5 files changed, 45 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index c041a2ae9af0..aebd456edec1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4272,7 +4272,8 @@ i915_max_freq_set(void *data, u64 val)
 
dev_priv->rps.max_freq_softlimit = val;
 
-   intel_set_rps(dev_priv, val);
+   if (intel_set_rps(dev_priv, val))
+   DRM_DEBUG_DRIVER("failed to update RPS to new softlimit\n");
 
mutex_unlock(_priv->rps.hw_lock);
 
@@ -4327,7 +4328,8 @@ i915_min_freq_set(void *data, u64 val)
 
dev_priv->rps.min_freq_softlimit = val;
 
-   intel_set_rps(dev_priv, val);
+   if (intel_set_rps(dev_priv, val))
+   DRM_DEBUG_DRIVER("failed to update RPS to new softlimit\n");
 
mutex_unlock(_priv->rps.hw_lock);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b230184e43e4..47acfa9d4b8e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3753,7 +3753,7 @@ extern void i915_redisable_vga(struct drm_i915_private 
*dev_priv);
 extern void i915_redisable_vga_power_on(struct drm_i915_private *dev_priv);
 extern bool ironlake_set_drps(struct drm_i915_private *dev_priv, u8 val);
 extern void intel_init_pch_refclk(struct drm_i915_private *dev_priv);
-extern void intel_set_rps(struct drm_i915_private *dev_priv, u8 val);
+extern int intel_set_rps(struct drm_i915_private *dev_priv, u8 val);
 extern bool intel_set_memory_cxsr(struct drm_i915_private *dev_priv,
  bool enable);
 
@@ -3779,7 +3779,7 @@ int skl_pcode_request(struct drm_i915_private *dev_priv, 
u32 mbox, u32 request,
 
 /* intel_sideband.c */
 u32 vlv_punit_read(struct drm_i915_private *dev_priv, u32 addr);
-void vlv_punit_write(struct drm_i915_private *dev_priv, u32 addr, u32 val);
+int vlv_punit_write(struct drm_i915_private *dev_priv, u32 addr, u32 val);
 u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr);
 u32 vlv_iosf_sb_read(struct drm_i915_private *dev_priv, u8 port, u32 reg);
 void vlv_iosf_sb_write(struct drm_i915_private *dev_priv, u8 port, u32 reg, 
u32 val);
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c 
b/drivers/gpu/drm/i915/i915_sysfs.c
index 376ac957cd1c..a721ff116101 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -395,13 +395,13 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
/* We still need *_set_rps to process the new max_delay and
 * update the interrupt limits and PMINTRMSK even though
 * frequency request may be unchanged. */
-   intel_set_rps(dev_priv, val);
+   ret = intel_set_rps(dev_priv, val);
 
mutex_unlock(_priv->rps.hw_lock);
 
intel_runtime_pm_put(dev_priv);
 
-   return count;
+   return ret ?: count;
 }
 
 static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct 
device_attribute *attr, char *buf)
@@ -448,14 +448,13 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
/* We still need *_set_rps to process the new min_delay and
 * update the interrupt limits and PMINTRMSK even though
 * frequency request may be unchanged. */
-   intel_set_rps(dev_priv, val);
+   ret = intel_set_rps(dev_priv, val);
 
mutex_unlock(_priv->rps.hw_lock);
 
intel_runtime_pm_put(dev_priv);
 
-   return count;
-
+   return ret ?: count;
 }
 
 static DEVICE_ATTR(gt_act_freq_mhz, S_IRUGO, gt_act_freq_mhz_show, NULL);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 21cdf8d73259..a58c0edd7578 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4933,11 +4933,11 @@ static u32 gen6_rps_pm_mask(struct drm_i915_private 
*dev_priv, u8 val)
 /* gen6_set_rps is called to update the frequency request, but should also be
  * called when the range (min_delay and max_delay) is modified so that we can
  * update the GEN6_RP_INTERRUPT_LIMITS register accordingly. */
-static void gen6_set_rps(struct drm_i915_private *dev_priv, u8 val)
+static int gen6_set_rps(struct drm_i915_private *dev_priv, u8 val)
 {
/* WaGsvDisableTurbo: Workaround to disable turbo on BXT A* */
if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_A1))
-   return;
+   return 0;
 
   

Re: [Intel-gfx] [PATCH] drm/i915/byt: Avoid tweaking evaluation thresholds

[Mika Kuoppala]

Chris Wilson  writes:

> On Wed, Jan 25, 2017 at 03:09:04PM +0200, Mika Kuoppala wrote:
>> Chris Wilson  writes:
>> 
>> > On Wed, Jan 25, 2017 at 02:31:08PM +0200, Mika Kuoppala wrote:
>> >> Certain Baytrails, namely the 4 cpu core variants, have been
>> >> plaqued by spurious system hangs, mostly occurring with light loads.
>> >> 
>> >> Multiple bisects by various people point to a commit which changes the
>> >> reclocking strategy for Baytrail to follow its bigger brethen:
>> >> commit 8fb55197e64d ("drm/i915: Agressive downclocking on Baytrail")
>> >> 
>> >> There is also a review comment attached to this commit from Deepak S
>> >> on avoiding punit access on Cherryview and thus it is excluded on
>> >> common reclocking path. By taking the same approach and omitting
>> >> the punit access by not tweaking the thresholds when the hardware
>> >> has been asked to move into different frequency, considerable gains
>> >> in stability have been observed.
>> >> 
>> >> With J1900 box, light render/video load would end up in system hang
>> >> in usually less than 12 hours. With this patch applied, the cumulative
>> >> uptime has now been 34 days without issues. To provoke system hang,
>> >> light loads on both render and bsd engines in parallel have been used:
>> >> glxgears >/dev/null 2>/dev/null &
>> >> mpv --vo=vaapi --hwdec=vaapi --loop=inf vid.mp4
>> >> 
>> >> So far, author has not witnessed system hang with above load
>> >> and this patch applied. Reports from the tenacious people at
>> >> kernel bugzilla are also promising.
>> >> 
>> >> Considering that the punit access frequency with this patch is
>> >> considerably less, there is a possibility that this will push
>> >> the, still unknown, root cause past the triggering point on most loads.
>> >> Further work on investigating the punit accesses on byt is welcomed.
>> >
>> > Please find the underlying problem and not disabling rps for all vlv
>> > for a GT specific problem.
>> 
>> This is not disabling rps.
>
> Your are disabling the key ingredients of the algorithm, making it less
> generic in order to workaround a problem elsewhere. You are tackling the
> symptoms and not the cause.

Yes, definitely we are tackling the symptoms.

We have been trying to find the root cause for 2 years.
Admittely hindered by the multiple other causes for
system hangs on baytrail platform.

One could argue that why was the deviation for Cherryview accepted,
as this just mimics the same way, omitting the sw adjustments.

It allows baytrail users to run their rigs without
intel_idle.max_cstate=1 which kind of ruins their power budget by far
bigger margin than this patch does.

-Mika

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

Re: [Intel-gfx] [PATCH v2 37/38] drm/i915: Add mock exercise for i915_gem_gtt_insert

[Joonas Lahtinen]

On to, 2017-01-19 at 11:41 +, Chris Wilson wrote:
> i915_gem_gtt_insert should allocate from the available free space in the
> GTT, evicting as necessary to create space.
> 
> Signed-off-by: Chris Wilson 



> + list_add(>batch_pool_link, );

Still dislike the obscurity. The least you could do is make a comment
on abusing the variable.

> +
> + /* And then force evictions */
> + for (total = 0;
> +  total + 2*I915_GTT_PAGE_SIZE <= i915->ggtt.base.total;
> +  total += 2*I915_GTT_PAGE_SIZE) {
> + struct i915_vma *vma;



> + err = i915_gem_gtt_insert(>ggtt.base, >node,
> +    obj->base.size, 0, obj->cache_level,
> +    0, i915->ggtt.base.total,
> +    0);
> + if (err) {
> + pr_err("i915_gem_gtt_insert (pass 1) failed at 
> %llu/%llu with err=%d\n",

pass 3?

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 36/38] drm/i915: Add mock exercise for i915_gem_gtt_reserve

[Joonas Lahtinen]

On to, 2017-01-19 at 11:41 +, Chris Wilson wrote:
> i915_gem_gtt_reserve should put the node exactly as requested in the
> GTT, evicting as required.
> 
> Signed-off-by: Chris Wilson 

I might de-magic 2*I915_GTT_PAGE_SIZE and couple of expect helpers, but
regardless;

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/byt: Avoid tweaking evaluation thresholds

[Patchwork]

== Series Details ==

Series: drm/i915/byt: Avoid tweaking evaluation thresholds
URL   : https://patchwork.freedesktop.org/series/18543/
State : success

== Summary ==

Series 18543v1 drm/i915/byt: Avoid tweaking evaluation thresholds
https://patchwork.freedesktop.org/api/1.0/series/18543/revisions/1/mbox/


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

396d17a6de32b4ef6cf1b531248e25ca6efe8001 drm-tip: 2017y-01m-25d-11h-07m-11s UTC 
integration manifest
75cf950 drm/i915/byt: Avoid tweaking evaluation thresholds

== Logs ==

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

Re: [Intel-gfx] [PATCH] drm/i915/byt: Avoid tweaking evaluation thresholds

[Chris Wilson]

On Wed, Jan 25, 2017 at 03:09:04PM +0200, Mika Kuoppala wrote:
> Chris Wilson  writes:
> 
> > On Wed, Jan 25, 2017 at 02:31:08PM +0200, Mika Kuoppala wrote:
> >> Certain Baytrails, namely the 4 cpu core variants, have been
> >> plaqued by spurious system hangs, mostly occurring with light loads.
> >> 
> >> Multiple bisects by various people point to a commit which changes the
> >> reclocking strategy for Baytrail to follow its bigger brethen:
> >> commit 8fb55197e64d ("drm/i915: Agressive downclocking on Baytrail")
> >> 
> >> There is also a review comment attached to this commit from Deepak S
> >> on avoiding punit access on Cherryview and thus it is excluded on
> >> common reclocking path. By taking the same approach and omitting
> >> the punit access by not tweaking the thresholds when the hardware
> >> has been asked to move into different frequency, considerable gains
> >> in stability have been observed.
> >> 
> >> With J1900 box, light render/video load would end up in system hang
> >> in usually less than 12 hours. With this patch applied, the cumulative
> >> uptime has now been 34 days without issues. To provoke system hang,
> >> light loads on both render and bsd engines in parallel have been used:
> >> glxgears >/dev/null 2>/dev/null &
> >> mpv --vo=vaapi --hwdec=vaapi --loop=inf vid.mp4
> >> 
> >> So far, author has not witnessed system hang with above load
> >> and this patch applied. Reports from the tenacious people at
> >> kernel bugzilla are also promising.
> >> 
> >> Considering that the punit access frequency with this patch is
> >> considerably less, there is a possibility that this will push
> >> the, still unknown, root cause past the triggering point on most loads.
> >> Further work on investigating the punit accesses on byt is welcomed.
> >
> > Please find the underlying problem and not disabling rps for all vlv
> > for a GT specific problem.
> 
> This is not disabling rps.

Your are disabling the key ingredients of the algorithm, making it less
generic in order to workaround a problem elsewhere. You are tackling the
symptoms and not the cause.
-Chris

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

Re: [Intel-gfx] [PATCH] drm/i915/byt: Avoid tweaking evaluation thresholds

[Mika Kuoppala]

Chris Wilson  writes:

> On Wed, Jan 25, 2017 at 02:31:08PM +0200, Mika Kuoppala wrote:
>> Certain Baytrails, namely the 4 cpu core variants, have been
>> plaqued by spurious system hangs, mostly occurring with light loads.
>> 
>> Multiple bisects by various people point to a commit which changes the
>> reclocking strategy for Baytrail to follow its bigger brethen:
>> commit 8fb55197e64d ("drm/i915: Agressive downclocking on Baytrail")
>> 
>> There is also a review comment attached to this commit from Deepak S
>> on avoiding punit access on Cherryview and thus it is excluded on
>> common reclocking path. By taking the same approach and omitting
>> the punit access by not tweaking the thresholds when the hardware
>> has been asked to move into different frequency, considerable gains
>> in stability have been observed.
>> 
>> With J1900 box, light render/video load would end up in system hang
>> in usually less than 12 hours. With this patch applied, the cumulative
>> uptime has now been 34 days without issues. To provoke system hang,
>> light loads on both render and bsd engines in parallel have been used:
>> glxgears >/dev/null 2>/dev/null &
>> mpv --vo=vaapi --hwdec=vaapi --loop=inf vid.mp4
>> 
>> So far, author has not witnessed system hang with above load
>> and this patch applied. Reports from the tenacious people at
>> kernel bugzilla are also promising.
>> 
>> Considering that the punit access frequency with this patch is
>> considerably less, there is a possibility that this will push
>> the, still unknown, root cause past the triggering point on most loads.
>> Further work on investigating the punit accesses on byt is welcomed.
>
> Please find the underlying problem and not disabling rps for all vlv
> for a GT specific problem.

This is not disabling rps.
-Mika

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

Re: [Intel-gfx] [PATCH 07/15] drm/gma500: Nuke device_is_agp callback

[Gustavo Padovan]

Hi Daniel,

2017-01-25 Daniel Vetter :

> Returning 0 for an on-chip gpu doesn't change anything at all.
> 
> Cc: Patrik Jakobsson 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/gma500/psb_drv.c | 6 --
>  1 file changed, 6 deletions(-)

Reviewed-by: Gustavo Padovan 

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

Re: [Intel-gfx] [PATCH 06/15] drm/doc: Clarify connector overview

[Gustavo Padovan]

Hi Daniel,

2017-01-25 Daniel Vetter :

> There was a bit of mix-up between initialization and registering.
> 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_connector.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index dd720d4cb4f7..c75ab242f907 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -40,11 +40,10 @@
>   *
>   * KMS driver must create, initialize, register and attach at a 
>   * drm_connector for each such sink. The instance is created as other KMS
> - * objects and initialized by setting the following fields.
> - *
> - * The connector is then registered with a call to drm_connector_init() with 
> a
> - * pointer to the connector functions and a connector type, and exposed 
> through
> - * sysfs with a call to drm_connector_register().
> + * objects and initialized by setting the following fields. The connector is
> + * initialized with a call to drm_connector_init() with a pointer to the
> + * connector functions and a connector type, and then exposed to userspace 
> with

_connector_funcs

Other than that:

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

[Intel-gfx] Broken DPMS with DisplayPort on CHV (also BYT?)

[Takashi Iwai]

Hi,

we've got a bug report about the blank monitor on Cherry Trail
machines.  And, Intel team (Cc'ed) spotted out that this seems
triggered by DPMS transition.  A patch like below actually fixes the
problem.

Of course it doesn't look like a right "fix".  Do you guys have any
hint for further debugging?

Some more (not alt-) facts:

- The machine we've tested is a Cherry Trail based box with two
  DisplayPorts.  Intel team mentioned that there are other machines
  suffering from the same problem.

- With some monitors, the screen remains blank during boot and on X.
  This can be cured after "xset dpms force off" and resume, or
  reconfiguring via xrandr.

- The issue seems specific to DP connections.  When two (identical)
  monitors are connected, only one monitor goes blank.

- The issue happens only with some monitor models (Dell E-series).
  Other Dell monitors (e.g. U-series) or other vendors seem working as
  far as we've tested.

- Intel team mentioned that a similar issue was seen on a Baytrail
  machine.

- The register value seems passed correctly.  The read back after the
  register write showed the expected value.
  Also, spinning more times in DPMS_ON loop didn't help, too.

- The issue is reproduced with the recent kernels (at least 4.9.x)


Any comments / suggestions appreciated.

thanks,

Takashi

--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2205,8 +2205,12 @@ void intel_dp_sink_dpms(struct intel_dp
return;
 
if (mode != DRM_MODE_DPMS_ON) {
+#if 0
ret = drm_dp_dpcd_writeb(_dp->aux, DP_SET_POWER,
 DP_SET_POWER_D3);
+#else
+   ret = 1;
+#endif
} else {
/*
 * When turning on, we need to retry for 1ms to give the sink
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 05/15] drm/core: Use recommened kerneldoc for struct member refs

[Gustavo Padovan]

Hi Daniel,

2017-01-25 Daniel Vetter :

> I just learned that _name.member_name works and looks pretty
> even. It doesn't (yet) link to the member directly though, which would
> be really good for big structures or vfunc tables (where the
> per-member kerneldoc tends to be long).
> 
> Also some minor drive-by polish where it makes sense, I read a lot
> of docs ...
> 
> Cc: Jani Nikula 
> Cc: Chris Wilson 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_auth.c |  4 ++--
>  drivers/gpu/drm/drm_drv.c  |  8 
>  drivers/gpu/drm/drm_fops.c | 11 ++-
>  drivers/gpu/drm/drm_irq.c  | 10 +-
>  drivers/gpu/drm/drm_pci.c  |  2 +-
>  drivers/gpu/drm/drm_platform.c |  2 +-
>  drivers/gpu/drm/drm_sysfs.c|  2 +-
>  include/drm/drm_auth.h | 12 ++--
>  include/drm/drm_drv.h  |  2 +-
>  include/drm/drm_irq.h  |  4 ++--
>  10 files changed, 29 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
> index 860cfe124c2a..7ff697389d74 100644
> --- a/drivers/gpu/drm/drm_auth.c
> +++ b/drivers/gpu/drm/drm_auth.c
> @@ -40,8 +40,8 @@
>   * least once successfully became the device master (either through the
>   * SET_MASTER IOCTL, or implicitly through opening the primary device node 
> when
>   * no one else is the current master that time) there exists one _master.
> - * This is noted in the is_master member of _file. All other clients have
> - * just a pointer to the _master they are associated with.
> + * This is noted in _file.is_master. All other clients have just a 
> pointer
> + * to the _master they are associated with.
>   *
>   * In addition only one _master can be the current master for a 
> _device.
>   * It can be switched through the DROP_MASTER and SET_MASTER IOCTL, or
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 72116978ec06..242f67c08982 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -309,7 +309,7 @@ void drm_minor_release(struct drm_minor *minor)
>   * userspace the device instance can be published using drm_dev_register().
>   *
>   * There is also deprecated support for initalizing device instances using
> - * bus-specific helpers and the ->load() callback. But due to
> + * bus-specific helpers and the _driver.load callback. But due to
>   * backwards-compatibility needs the device instance have to be published too
>   * early, which requires unpretty global locking to make safe and is 
> therefore
>   * only support for existing drivers not yet converted to the new scheme.
> @@ -720,9 +720,9 @@ static void remove_compat_control_link(struct drm_device 
> *dev)
>   * Never call this twice on any device!
>   *
>   * NOTE: To ensure backward compatibility with existing drivers method this
> - * function calls the ->load() method after registering the device nodes,
> - * creating race conditions. Usage of the ->load() methods is therefore
> - * deprecated, drivers must perform all initialization before calling
> + * function calls the _driver.load method after registering the device
> + * nodes, creating race conditions. Usage of the _driver.load methods is
> + * therefore deprecated, drivers must perform all initialization before 
> calling
>   * drm_dev_register().
>   *
>   * RETURNS:
> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index e22645375e60..afdf5b147f39 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -580,7 +580,7 @@ EXPORT_SYMBOL(drm_poll);
>   * kmalloc and @p must be the first member element.
>   *
>   * This is the locked version of drm_event_reserve_init() for callers which
> - * already hold dev->event_lock.
> + * already hold _device.event_lock.
>   *
>   * RETURNS:
>   *
> @@ -621,7 +621,7 @@ EXPORT_SYMBOL(drm_event_reserve_init_locked);
>   * If callers embedded @p into a larger structure it must be allocated with
>   * kmalloc and @p must be the first member element.
>   *
> - * Callers which already hold dev->event_lock should use
> + * Callers which already hold _device.event_lock should use
>   * drm_event_reserve_init_locked() instead.
>   *
>   * RETURNS:
> @@ -677,7 +677,7 @@ EXPORT_SYMBOL(drm_event_cancel_free);
>   *
>   * This function sends the event @e, initialized with 
> drm_event_reserve_init(),
>   * to its associated userspace DRM file. Callers must already hold
> - * dev->event_lock, see drm_send_event() for the unlocked version.
> + * _device.event_lock, see drm_send_event() for the unlocked version.
>   *
>   * Note that the core will take care of unlinking and disarming events when 
> the
>   * corresponding DRM file is closed. Drivers need not worry about whether the
> @@ -717,8 +717,9 @@ EXPORT_SYMBOL(drm_send_event_locked);
>   * @e: DRM event to deliver
>   *
>   * This function sends the event @e, 

Re: [Intel-gfx] [PATCH 04/15] drm/gem|prime|mm: Use recommened kerneldoc for struct member refs

[Gustavo Padovan]

Hi Daniel,

2017-01-25 Daniel Vetter :

> I just learned that _name.member_name works and looks pretty
> even. It doesn't (yet) link to the member directly though, which would
> be really good for big structures or vfunc tables (where the
> per-member kerneldoc tends to be long).
> 
> Also some minor drive-by polish where it makes sense, I read a lot
> of docs ...
> 
> Cc: Jani Nikula 
> Cc: Chris Wilson 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_gem.c   | 24 
>  drivers/gpu/drm/drm_mm.c|  4 ++--
>  drivers/gpu/drm/drm_prime.c |  2 +-
>  include/drm/drm_gem.h   | 16 
>  4 files changed, 23 insertions(+), 23 deletions(-)

Reviewed-by: Gustavo Padovan 

Gustavo

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

Re: [Intel-gfx] [PATCH i-g-t v2 19/33] tests/kms_plane: Add support for dynamic number of planes

[Mika Kahola]

On Tue, 2017-01-24 at 18:33 -0500, Robert Foss wrote:
> Add changes reflecting the new support for dynamic number of planes
> per pipe.
> 
> Signed-off-by: Robert Foss 
> ---
>  tests/kms_plane.c | 94 ++---
> --
>  1 file changed, 52 insertions(+), 42 deletions(-)
> 
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index e843a170..14c444fc 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -150,7 +150,7 @@ enum {
>  static void
>  test_plane_position_with_output(data_t *data,
>   enum pipe pipe,
> - enum igt_plane plane,
> + int plane,
>   igt_output_t *output,
>   unsigned int flags)
>  {
> @@ -170,7 +170,7 @@ test_plane_position_with_output(data_t *data,
>   igt_output_set_pipe(output, pipe);
>  
>   mode = igt_output_get_mode(output);
> - primary = igt_output_get_plane(output, 0);
> + primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
>   sprite = igt_output_get_plane(output, plane);
>  
>   create_fb_for_mode__position(data, mode, 100, 100, 64, 64,
> @@ -222,7 +222,7 @@ test_plane_position_with_output(data_t *data,
>  }
>  
>  static void
> -test_plane_position(data_t *data, enum pipe pipe, enum igt_plane
> plane,
> +test_plane_position(data_t *data, enum pipe pipe, int plane,
>   unsigned int flags)
>  {
>   igt_output_t *output;
> @@ -294,7 +294,7 @@ enum {
>  static void
>  test_plane_panning_with_output(data_t *data,
>      enum pipe pipe,
> -    enum igt_plane plane,
> +    int plane,
>      igt_output_t *output,
>      unsigned int flags)
>  {
> @@ -348,7 +348,7 @@ test_plane_panning_with_output(data_t *data,
>  }
>  
>  static void
> -test_plane_panning(data_t *data, enum pipe pipe, enum igt_plane
> plane,
> +test_plane_panning(data_t *data, enum pipe pipe, int plane,
>  unsigned int flags)
misaligned parameter. With that fixed, this is

Reviewed-by: Mika Kahola 

>  {
>   igt_output_t *output;
> @@ -367,47 +367,57 @@ test_plane_panning(data_t *data, enum pipe
> pipe, enum igt_plane plane,
>  }
>  
>  static void
> -run_tests_for_pipe_plane(data_t *data, enum pipe pipe, enum
> igt_plane plane)
> +run_tests_for_pipe_plane(data_t *data, enum pipe pipe)
>  {
> - igt_subtest_f("plane-position-covered-pipe-%s-plane-%d",
> -   kmstest_pipe_name(pipe), plane)
> - test_plane_position(data, pipe, plane,
> - TEST_POSITION_FULLY_COVERED);
> -
> - igt_subtest_f("plane-position-hole-pipe-%s-plane-%d",
> -   kmstest_pipe_name(pipe), plane)
> - test_plane_position(data, pipe, plane, 0);
> -
> - igt_subtest_f("plane-position-hole-dpms-pipe-%s-plane-%d",
> -   kmstest_pipe_name(pipe), plane)
> - test_plane_position(data, pipe, plane,
> - TEST_DPMS);
> -
> - igt_subtest_f("plane-panning-top-left-pipe-%s-plane-%d",
> -   kmstest_pipe_name(pipe), plane)
> - test_plane_panning(data, pipe, plane,
> TEST_PANNING_TOP_LEFT);
> -
> - igt_subtest_f("plane-panning-bottom-right-pipe-%s-plane-%d",
> -   kmstest_pipe_name(pipe), plane)
> - test_plane_panning(data, pipe, plane,
> -    TEST_PANNING_BOTTOM_RIGHT);
> -
> - igt_subtest_f("plane-panning-bottom-right-suspend-pipe-%s-
> plane-%d",
> -   kmstest_pipe_name(pipe), plane)
> - test_plane_panning(data, pipe, plane,
> -    TEST_PANNING_BOTTOM_RIGHT |
> -    TEST_SUSPEND_RESUME);
> -}
> + igt_subtest_f("plane-position-covered-pipe-%s-planes",
> +   kmstest_pipe_name(pipe)) {
> + int n_planes = data->display.pipes[pipe].n_planes;
> + for (int plane = 1; plane < n_planes; plane++)
> + test_plane_position(data, pipe, plane,
> + TEST_POSITION_FULLY_COVE
> RED);
> + }
>  
> -static void
> -run_tests_for_pipe(data_t *data, enum pipe pipe)
> -{
> - int plane;
> + igt_subtest_f("plane-position-hole-pipe-%s-planes",
> +   kmstest_pipe_name(pipe)) {
> + int n_planes = data->display.pipes[pipe].n_planes;
> + for (int plane = 1; plane < n_planes; plane++)
> + test_plane_position(data, pipe, plane, 0);
> + }
> +
> + igt_subtest_f("plane-position-hole-dpms-pipe-%s-planes",
> +   kmstest_pipe_name(pipe)) {
> + int n_planes = data->display.pipes[pipe].n_planes;
> + for (int plane = 1; 

Re: [Intel-gfx] [PATCH 01/15] drm/kms-helpers: Use recommened kerneldoc for struct member refs

[Gustavo Padovan]

Hi Daniel,

2017-01-25 Daniel Vetter :

> I just learned that _name.member_name works and looks pretty
> even. It doesn't (yet) link to the member directly though, which would
> be really good for big structures or vfunc tables (where the
> per-member kerneldoc tends to be long).
> 
> Also some minor drive-by polish where it makes sense, I read a lot
> of docs ...
> 
> Cc: Jani Nikula 
> Cc: Chris Wilson 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_atomic_helper.c  |  97 ++--
>  drivers/gpu/drm/drm_crtc_helper.c|  28 +++---
>  drivers/gpu/drm/drm_dp_helper.c  |   2 +-
>  drivers/gpu/drm/drm_fb_helper.c  |  48 +-
>  drivers/gpu/drm/drm_plane_helper.c   |   9 +-
>  drivers/gpu/drm/drm_probe_helper.c   |  14 +--
>  include/drm/drm_atomic_helper.h  |  13 +--
>  include/drm/drm_dp_mst_helper.h  |   7 +-
>  include/drm/drm_flip_work.h  |   2 +-
>  include/drm/drm_modeset_helper_vtables.h | 146 
> ---
>  include/drm/drm_simple_kms_helper.h  |  12 +--
>  11 files changed, 197 insertions(+), 181 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 1f0cd7e715c9..5e512dd3a2c4 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -458,22 +458,25 @@ mode_fixup(struct drm_atomic_state *state)
>   * Check the state object to see if the requested state is physically 
> possible.
>   * This does all the crtc and connector related computations for an atomic
>   * update and adds any additional connectors needed for full modesets and 
> calls
> - * down into ->mode_fixup functions of the driver backend.
> - *
> - * crtc_state->mode_changed is set when the input mode is changed.
> - * crtc_state->connectors_changed is set when a connector is added or
> - * removed from the crtc.
> - * crtc_state->active_changed is set when crtc_state->active changes,
> - * which is used for dpms.
> + * down into _crtc_helper_funcs.mode_fixup and
> + * _encoder_helper_funcs.mode_fixup or
> + * _encoder_helper_funcs.atomic_check functions of the driver backend.
> + *
> + * _crtc_state.mode_changed is set when the input mode is changed.
> + * _crtc_state.connectors_changed is set when a connector is added or
> + * removed from the crtc.  _crtc_state.active_changed is set when
> + * _crtc_state.active changes, which is used for DPMS.
>   * See also: drm_atomic_crtc_needs_modeset()
>   *
>   * IMPORTANT:
>   *
> - * Drivers which set ->mode_changed (e.g. in their ->atomic_check hooks if a
> - * plane update can't be done without a full modeset) _must_ call this 
> function
> - * afterwards after that change. It is permitted to call this function 
> multiple
> - * times for the same update, e.g. when the ->atomic_check functions depend 
> upon
> - * the adjusted dotclock for fifo space allocation and watermark computation.
> + * Drivers which set _crtc_state.mode_changed (e.g. in their
> + * _plane_helper_funcs.atomic_check hooks if a plane update can't be done
> + * without a full modeset) _must_ call this function afterwards after that
> + * change. It is permitted to call this function multiple times for the same
> + * update, e.g. when the _crtc_helper_funcs.atomic_check functions depend
> + * upon the adjusted dotclock for fifo space allocation and watermark
> + * computation.
>   *
>   * RETURNS:
>   * Zero for success or -errno
> @@ -584,9 +587,10 @@ EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
>   *
>   * Check the state object to see if the requested state is physically 
> possible.
>   * This does all the plane update related checks using by calling into the
> - * ->atomic_check hooks provided by the driver.
> + * _crtc_helper_funcs.atomic_check and 
> _plane_helper_funcs.atomic_check
> + * hooks provided by the driver.
>   *
> - * It also sets crtc_state->planes_changed to indicate that a crtc has
> + * It also sets _crtc_state.planes_changed to indicate that a crtc has
>   * updated planes.
>   *
>   * RETURNS:
> @@ -648,14 +652,15 @@ EXPORT_SYMBOL(drm_atomic_helper_check_planes);
>   * Check the state object to see if the requested state is physically 
> possible.
>   * Only crtcs and planes have check callbacks, so for any additional (global)
>   * checking that a driver needs it can simply wrap that around this function.
> - * Drivers without such needs can directly use this as their ->atomic_check()
> - * callback.
> + * Drivers without such needs can directly use this as their
> + * _mode_config_funcs.atomic_check callback.
>   *
>   * This just wraps the two parts of the state checking for planes and modeset
>   * state in the default order: First it calls 
> drm_atomic_helper_check_modeset()
>   * and then drm_atomic_helper_check_planes(). The assumption is that the
> - * 

Re: [Intel-gfx] [PATCH] drm/i915/byt: Avoid tweaking evaluation thresholds

[Chris Wilson]

On Wed, Jan 25, 2017 at 02:31:08PM +0200, Mika Kuoppala wrote:
> Certain Baytrails, namely the 4 cpu core variants, have been
> plaqued by spurious system hangs, mostly occurring with light loads.
> 
> Multiple bisects by various people point to a commit which changes the
> reclocking strategy for Baytrail to follow its bigger brethen:
> commit 8fb55197e64d ("drm/i915: Agressive downclocking on Baytrail")
> 
> There is also a review comment attached to this commit from Deepak S
> on avoiding punit access on Cherryview and thus it is excluded on
> common reclocking path. By taking the same approach and omitting
> the punit access by not tweaking the thresholds when the hardware
> has been asked to move into different frequency, considerable gains
> in stability have been observed.
> 
> With J1900 box, light render/video load would end up in system hang
> in usually less than 12 hours. With this patch applied, the cumulative
> uptime has now been 34 days without issues. To provoke system hang,
> light loads on both render and bsd engines in parallel have been used:
> glxgears >/dev/null 2>/dev/null &
> mpv --vo=vaapi --hwdec=vaapi --loop=inf vid.mp4
> 
> So far, author has not witnessed system hang with above load
> and this patch applied. Reports from the tenacious people at
> kernel bugzilla are also promising.
> 
> Considering that the punit access frequency with this patch is
> considerably less, there is a possibility that this will push
> the, still unknown, root cause past the triggering point on most loads.
> Further work on investigating the punit accesses on byt is welcomed.

Please find the underlying problem and not disabling rps for all vlv
for a GT specific problem.
-Chris

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

Re: [Intel-gfx] [PATCH i-g-t v2 18/33] tests/kms_pipe_color: Add support for dynamic number of planes

[Mika Kahola]

Reviewed-by: Mika Kahola 

On Tue, 2017-01-24 at 18:33 -0500, Robert Foss wrote:
> Add changes reflecting the new support for dynamic number of planes
> per pipe.
> 
> Signed-off-by: Robert Foss 
> ---
>  tests/kms_pipe_color.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/kms_pipe_color.c b/tests/kms_pipe_color.c
> index 1aff7d54..c7a5d2f1 100644
> --- a/tests/kms_pipe_color.c
> +++ b/tests/kms_pipe_color.c
> @@ -857,9 +857,9 @@ run_tests_for_pipe(data_t *data, enum pipe p)
>   igt_require(p < data->display.n_pipes);
>  
>   pipe = >display.pipes[p];
> - igt_require(pipe->n_planes >= IGT_PLANE_PRIMARY);
> + igt_require(pipe->n_planes >= 0);
>  
> - primary = >planes[IGT_PLANE_PRIMARY];
> + primary = igt_pipe_get_plane_type(pipe,
> DRM_PLANE_TYPE_PRIMARY);
>  
>   data->pipe_crc = igt_pipe_crc_new(primary->pipe-
> >pipe,
>     INTEL_PIPE_CRC_SOU
> RCE_AUTO);
-- 
Mika Kahola - Intel OTC

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

Re: [Intel-gfx] [PATCH 02/15] drm/bridge: Use recommened kerneldoc for struct member refs

[Daniel Vetter]

On Wed, Jan 25, 2017 at 03:03:42PM +0530, Archit Taneja wrote:
> 
> 
> On 01/25/2017 11:56 AM, Daniel Vetter wrote:
> > I just learned that _name.member_name works and looks pretty
> > even. It doesn't (yet) link to the member directly though, which would
> > be really good for big structures or vfunc tables (where the
> > per-member kerneldoc tends to be long).
> > 
> > Also some minor drive-by polish where it makes sense, I read a lot
> > of docs ...
> 
> This seems like a leftover from the older version of the patch, which
> we decided to not take. I guess we could drop it.

Right, this was a rebase oversight, thanks a lot for catching it. I'l drop
it now to make sure it won't resurface :-)
-Daniel

> 
> Archit
> 
> > 
> > Cc: Archit Taneja 
> > Cc: Jani Nikula 
> > Cc: Chris Wilson 
> > Signed-off-by: Daniel Vetter 
> > ---
> >  include/drm/drm_bridge.h | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > index fdd82fcbf168..1595a57dfbf2 100644
> > --- a/include/drm/drm_bridge.h
> > +++ b/include/drm/drm_bridge.h
> > @@ -87,9 +87,9 @@ struct drm_bridge_funcs {
> >  * True if an acceptable configuration is possible, false if the modeset
> >  * operation should be rejected.
> >  */
> > -   bool (*mode_fixup)(struct drm_bridge *bridge,
> > -  const struct drm_display_mode *mode,
> > -  struct drm_display_mode *adjusted_mode);
> > +   bool (*mode_fixup)(struct drm_bridge *bridge, const struct
> > +  drm_display_mode *mode, struct drm_display_mode
> > +  *adjusted_mode);
> > /**
> >  * @disable:
> >  *
> > 
> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

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

Re: [Intel-gfx] [PATCH i-g-t v2 17/33] tests/kms_panel_fitting: Add support for dynamic number of planes

[Mika Kahola]

Reviewed-by: Mika Kahola 

On Tue, 2017-01-24 at 18:33 -0500, Robert Foss wrote:
> Add changes reflecting the new support for dynamic number of planes
> per pipe.
> 
> Signed-off-by: Robert Foss 
> ---
>  tests/kms_panel_fitting.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c
> index 1b350762..e145a2df 100644
> --- a/tests/kms_panel_fitting.c
> +++ b/tests/kms_panel_fitting.c
> @@ -76,10 +76,10 @@ static void prepare_crtc(data_t *data,
> igt_output_t *output, enum pipe pipe,
>    * there's no way (that works) to light up a pipe with only
> a sprite
>    * plane enabled at the moment.
>    */
> - if (!plane->is_primary) {
> + if (plane->type != DRM_PLANE_TYPE_PRIMARY) {
>   igt_plane_t *primary;
>  
> - primary = igt_output_get_plane(output,
> IGT_PLANE_PRIMARY);
> + primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
>   igt_plane_set_fb(primary, >fb1);
>   }
>  
> @@ -116,10 +116,10 @@ static void cleanup_crtc(data_t *data,
> igt_output_t *output, igt_plane_t *plane)
>   data->fb_id3 = 0;
>   }
>  
> - if (!plane->is_primary) {
> + if (plane->type != DRM_PLANE_TYPE_PRIMARY) {
>   igt_plane_t *primary;
>  
> - primary = igt_output_get_plane(output,
> IGT_PLANE_PRIMARY);
> + primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
>   igt_plane_set_fb(primary, NULL);
>   }
>  
> @@ -174,7 +174,7 @@ static void test_panel_fitting(data_t *d)
>   /* Set up display to enable panel fitting */
>   mode->hdisplay = 640;
>   mode->vdisplay = 480;
> - d->plane1 = igt_output_get_plane(output,
> IGT_PLANE_PRIMARY);
> + d->plane1 = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
>   prepare_crtc(d, output, pipe, d->plane1, mode,
> COMMIT_LEGACY);
>  
>   /* disable panel fitting */
> @@ -189,7 +189,7 @@ static void test_panel_fitting(data_t *d)
>   prepare_crtc(d, output, pipe, d->plane1,
> _mode, COMMIT_LEGACY);
>  
>   /* Set up fb2->plane2 mapping. */
> - d->plane2 = igt_output_get_plane(output,
> IGT_PLANE_2);
> + d->plane2 = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_OVERLAY);
>   igt_plane_set_fb(d->plane2, >fb2);
>  
>   /* enable sprite plane */
> @@ -226,8 +226,8 @@ test_panel_fitting_fastset(igt_display_t
> *display, const enum pipe pipe, igt_out
>   igt_output_override_mode(output, );
>   igt_output_set_pipe(output, pipe);
>  
> - primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
> - sprite = igt_output_get_plane(output, IGT_PLANE_2);
> + primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
> + sprite = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_OVERLAY);
>  
>   igt_create_color_fb(display->drm_fd, mode.hdisplay,
> mode.vdisplay,
>   DRM_FORMAT_XRGB,
> LOCAL_DRM_FORMAT_MOD_NONE,
-- 
Mika Kahola - Intel OTC

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915: ignore forcewake get/put when using vgpu

[Li, Weinan Z]

> -Original Message-
> From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> Sent: Wednesday, January 25, 2017 3:17 PM
> To: Li, Weinan Z 
> Cc: intel-gfx@lists.freedesktop.org; intel-gvt-...@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915: ignore forcewake get/put
> when using vgpu
> 
> On Wed, Jan 25, 2017 at 02:04:56PM +0800, Weinan Li wrote:
> > Host maintian the hardware's forcewake state, guest don't need and
> > also can't control it. Although vgpu_read/write bypass
> > forcewake_get/put in MMIO read/write, but still have separate path
> > called by "intel_uncore_forcewake_get/put" and
> > "intel_uncore_forcewake_get/put__locked". Unnecessary MMIO access in
> > guest waste much CPU cost. Since we full virtualize the MMIO, just
> > ignore the forcewake get/put in low level.
> 
> This patch is doing multiple things as you are altering a couple of 
> independent
> paths at the same time (fw control and mmio funcs).
> 
ok, let me separate 2 patches, 1 fw control, 2 remove vgpu_read/write funcs.
> >  static void
> > +vgpu_fw_domains_get(struct drm_i915_private *dev_priv,
> > +   enum forcewake_domains fw_domains) {
> > +   /* Guest driver doesn't need to takes care forcewake. */; }
> > +
> > +static void
> > +vgpu_fw_domains_put(struct drm_i915_private *dev_priv,
> > +   enum forcewake_domains fw_domains) {
> > +   /* Guest driver doesn't need to takes care forcewake. */; }
> 
> Or just 1 vgpu_fw_domains_nop()
vgpu_fw_domains_nop more simple and readable. 
> -Chris
> 
> --
> Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t v2 16/33] tests/kms_mmio_vs_cs_flip: Add support for dynamic number of planes

[Mika Kahola]

Reviewed-by: Mika Kahola 

On Tue, 2017-01-24 at 18:33 -0500, Robert Foss wrote:
> Add changes reflecting the new support for dynamic number of planes
> per pipe.
> 
> Signed-off-by: Robert Foss 
> ---
>  tests/kms_mmio_vs_cs_flip.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/kms_mmio_vs_cs_flip.c
> b/tests/kms_mmio_vs_cs_flip.c
> index 2f64f633..09217b31 100644
> --- a/tests/kms_mmio_vs_cs_flip.c
> +++ b/tests/kms_mmio_vs_cs_flip.c
> @@ -191,7 +191,7 @@ static void make_gpu_busy(data_t *data, uint32_t
> flip_handle)
>   * supposed to be.
>   */
>  static void
> -test_plane(data_t *data, igt_output_t *output, enum pipe pipe, enum
> igt_plane plane)
> +test_plane(data_t *data, igt_output_t *output, enum pipe pipe, int
> plane)
>  {
>   struct igt_fb red_fb, green_fb, blue_fb;
>   drmModeModeInfo *mode;
> @@ -201,7 +201,7 @@ test_plane(data_t *data, igt_output_t *output,
> enum pipe pipe, enum igt_plane pl
>  
>   igt_output_set_pipe(output, pipe);
>  
> - primary = igt_output_get_plane(output, 0);
> + primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
>   sprite = igt_output_get_plane(output, plane);
>  
>   mode = igt_output_get_mode(output);
> @@ -455,7 +455,7 @@ static void
>  run_plane_test(data_t *data)
>  {
>   igt_output_t *output;
> - enum igt_plane plane = 1; /* testing with one sprite is
> enough */
> + int plane = 1; /* testing with one sprite is enough */
>   int valid_tests = 0;
>   enum pipe pipe;
>  
-- 
Mika Kahola - Intel OTC

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

Re: [Intel-gfx] [PATCH i-g-t v2 15/33] tests/kms_mmap_write_crc: Add support for dynamic number of planes

[Mika Kahola]

Reviewed-by: Mika Kahola 

On Tue, 2017-01-24 at 18:33 -0500, Robert Foss wrote:
> Add changes reflecting the new support for dynamic number of planes
> per pipe.
> 
> Signed-off-by: Robert Foss 
> ---
>  tests/kms_mmap_write_crc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/kms_mmap_write_crc.c b/tests/kms_mmap_write_crc.c
> index eb8586d1..8a1331cc 100644
> --- a/tests/kms_mmap_write_crc.c
> +++ b/tests/kms_mmap_write_crc.c
> @@ -177,7 +177,7 @@ static void prepare_crtc(data_t *data)
>   DRM_FORMAT_XRGB,
> LOCAL_DRM_FORMAT_MOD_NONE,
>   1.0, 1.0, 1.0, >fb[0]);
>  
> - data->primary = igt_output_get_plane(output,
> IGT_PLANE_PRIMARY);
> + data->primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
>  
>   igt_plane_set_fb(data->primary, >fb[0]);
>   igt_display_commit(display);
-- 
Mika Kahola - Intel OTC

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

[Intel-gfx] [PATCH] drm/i915/byt: Avoid tweaking evaluation thresholds

[Mika Kuoppala]

Certain Baytrails, namely the 4 cpu core variants, have been
plaqued by spurious system hangs, mostly occurring with light loads.

Multiple bisects by various people point to a commit which changes the
reclocking strategy for Baytrail to follow its bigger brethen:
commit 8fb55197e64d ("drm/i915: Agressive downclocking on Baytrail")

There is also a review comment attached to this commit from Deepak S
on avoiding punit access on Cherryview and thus it is excluded on
common reclocking path. By taking the same approach and omitting
the punit access by not tweaking the thresholds when the hardware
has been asked to move into different frequency, considerable gains
in stability have been observed.

With J1900 box, light render/video load would end up in system hang
in usually less than 12 hours. With this patch applied, the cumulative
uptime has now been 34 days without issues. To provoke system hang,
light loads on both render and bsd engines in parallel have been used:
glxgears >/dev/null 2>/dev/null &
mpv --vo=vaapi --hwdec=vaapi --loop=inf vid.mp4

So far, author has not witnessed system hang with above load
and this patch applied. Reports from the tenacious people at
kernel bugzilla are also promising.

Considering that the punit access frequency with this patch is
considerably less, there is a possibility that this will push
the, still unknown, root cause past the triggering point on most loads.
Further work on investigating the punit accesses on byt is welcomed.

References: https://bugzilla.kernel.org/show_bug.cgi?id=109051
Cc: Chris Wilson 
Cc: Ville Syrjälä 
Cc: Len Brown 
Cc: Daniel Vetter 
Cc: Jani Nikula 
Cc: frit...@xbmc.org
Cc: m...@iki.fi
Cc: Ezequiel Garcia 
CC: Michal Feix 
Cc: Hans de Goede 
Cc: Deepak S 
Cc: Jarkko Nikula 
Cc:  # v4.2+
Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_irq.c | 4 ++--
 drivers/gpu/drm/i915/i915_reg.h | 2 ++
 drivers/gpu/drm/i915/intel_pm.c | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 3fc286c..4b9635f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1039,7 +1039,7 @@ static u32 vlv_wa_c0_ei(struct drm_i915_private 
*dev_priv, u32 pm_iir)
if (pm_iir & GEN6_PM_RP_DOWN_EI_EXPIRED) {
if (!vlv_c0_above(dev_priv,
  _priv->rps.down_ei, ,
- dev_priv->rps.down_threshold))
+ VLV_RP_DOWN_EI_THRESHOLD))
events |= GEN6_PM_RP_DOWN_THRESHOLD;
dev_priv->rps.down_ei = now;
}
@@ -1047,7 +1047,7 @@ static u32 vlv_wa_c0_ei(struct drm_i915_private 
*dev_priv, u32 pm_iir)
if (pm_iir & GEN6_PM_RP_UP_EI_EXPIRED) {
if (vlv_c0_above(dev_priv,
 _priv->rps.up_ei, ,
-dev_priv->rps.up_threshold))
+VLV_RP_UP_EI_THRESHOLD))
events |= GEN6_PM_RP_UP_THRESHOLD;
dev_priv->rps.up_ei = now;
}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 70d9616..09f6aea 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -787,6 +787,8 @@ enum skl_disp_power_wells {
 #defineCHV_BIAS_CPU_50_SOC_50 (3 << 2)
 
 #define VLV_CZ_CLOCK_TO_MILLI_SEC  10
+#define VLV_RP_UP_EI_THRESHOLD 90
+#define VLV_RP_DOWN_EI_THRESHOLD   70
 
 /* vlv2 north clock has */
 #define CCK_FUSE_REG   0x8
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index db24f89..1923b6b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4983,7 +4983,7 @@ static void valleyview_set_rps(struct drm_i915_private 
*dev_priv, u8 val)
 
if (val != dev_priv->rps.cur_freq) {
vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
-   if (!IS_CHERRYVIEW(dev_priv))
+   if (!(IS_CHERRYVIEW(dev_priv) || IS_VALLEYVIEW(dev_priv)))
gen6_set_rps_thresholds(dev_priv, val);
}
 
-- 
2.7.4

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

Re: [Intel-gfx] [PATCH i-g-t v2 14/33] tests/kms_legacy_colorkey: Add support for dynamic number of planes

[Mika Kahola]

Reviewed-by: Mika Kahola 

On Tue, 2017-01-24 at 18:33 -0500, Robert Foss wrote:
> Add changes reflecting the new support for dynamic number of planes
> per pipe.
> 
> Signed-off-by: Robert Foss 
> ---
>  tests/kms_legacy_colorkey.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/kms_legacy_colorkey.c
> b/tests/kms_legacy_colorkey.c
> index 25f98aad..150520ce 100644
> --- a/tests/kms_legacy_colorkey.c
> +++ b/tests/kms_legacy_colorkey.c
> @@ -55,8 +55,10 @@ igt_simple_main
>  
>   for_each_pipe(, p) {
>   for_each_plane_on_pipe(, p, plane) {
> + bool is_valid = (plane->type ==
> DRM_PLANE_TYPE_PRIMARY ||
> +  plane->type ==
> DRM_PLANE_TYPE_CURSOR);
>   test_plane(plane->drm_plane->plane_id,
> -    (plane->is_cursor || plane-
> >is_primary) ? -ENOENT : 0);
> +    is_valid ? -ENOENT : 0);
>  
>   max_id = max(max_id, plane->drm_plane-
> >plane_id);
>   }
-- 
Mika Kahola - Intel OTC

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

Re: [Intel-gfx] [PATCH i-g-t v2 12/33] tests/kms_fence_pin_leak: Add support for dynamic number of planes

[Mika Kahola]

Reviewed-by: Mika Kahola 

On Tue, 2017-01-24 at 18:33 -0500, Robert Foss wrote:
> Add changes reflecting the new support for dynamic number of planes
> per pipe.
> 
> Signed-off-by: Robert Foss 
> ---
>  tests/kms_fence_pin_leak.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/kms_fence_pin_leak.c b/tests/kms_fence_pin_leak.c
> index 6301e01a..8bbd5563 100644
> --- a/tests/kms_fence_pin_leak.c
> +++ b/tests/kms_fence_pin_leak.c
> @@ -115,7 +115,7 @@ static void run_single_test(data_t *data, enum
> pipe pipe, igt_output_t *output)
>   igt_output_set_pipe(output, pipe);
>  
>   mode = igt_output_get_mode(output);
> - primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
> + primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
>  
>   igt_create_color_fb(data->drm_fd, mode->hdisplay, mode-
> >vdisplay,
>   DRM_FORMAT_XRGB,
-- 
Mika Kahola - Intel OTC

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

Re: [Intel-gfx] [PATCH i-g-t v2 13/33] tests/kms_flip_event_leak: Add support for dynamic number of planes

[Mika Kahola]

Reviewed-by: Mika Kahola 

On Tue, 2017-01-24 at 18:33 -0500, Robert Foss wrote:
> Add changes reflecting the new support for dynamic number of planes
> per pipe.
> 
> Signed-off-by: Robert Foss 
> ---
>  tests/kms_flip_event_leak.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/kms_flip_event_leak.c
> b/tests/kms_flip_event_leak.c
> index f22af0f0..f1a8abd8 100644
> --- a/tests/kms_flip_event_leak.c
> +++ b/tests/kms_flip_event_leak.c
> @@ -50,7 +50,7 @@ static void test(data_t *data, enum pipe pipe,
> igt_output_t *output)
>   /* select the pipe we want to use */
>   igt_output_set_pipe(output, pipe);
>  
> - primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
> + primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
>   mode = igt_output_get_mode(output);
>  
>   igt_create_color_fb(data->drm_fd, mode->hdisplay, mode-
> >vdisplay,
-- 
Mika Kahola - Intel OTC

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

Re: [Intel-gfx] [PATCH i-g-t v2 11/33] tests/kms_fbc_crc: Add support for dynamic number of planes

[Mika Kahola]

On Tue, 2017-01-24 at 18:33 -0500, Robert Foss wrote:
> Add changes reflecting the new support for dynamic number of planes
> per pipe.
> 
> Signed-off-by: Robert Foss 
> ---
>  tests/kms_fbc_crc.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
> index a696e124..d2c76fb1 100644
> --- a/tests/kms_fbc_crc.c
> +++ b/tests/kms_fbc_crc.c
> @@ -372,7 +372,7 @@ static bool prepare_test(data_t *data, enum
> test_mode test_mode)
>   igt_output_t *output = data->output;
>   igt_pipe_crc_t *pipe_crc;
>  
> - data->primary = igt_output_get_plane(data->output,
> IGT_PLANE_PRIMARY);
> + data->primary = igt_output_get_plane_type(data->output,
> DRM_PLANE_TYPE_PRIMARY);
>  
>   create_fbs(data, true, data->fb);
>  
> @@ -457,10 +457,11 @@ static void finish_crtc(data_t *data, enum
> test_mode mode)
>  static void reset_display(data_t *data)
>  {
>   igt_display_t *display = >display;
> - enum pipe pipe;
> + enum pipe pipe_type;
Earlier patch defined this as pipe_id. For consistency, should we
rename this as pipe_id too? I'll leave this up to you. Either way, this
is 

Reviewed-by: Mika Kahola 
>  
> - for_each_pipe(display, pipe) {
> - igt_plane_t *plane = 
> >pipes[pipe].planes[IGT_PLANE_PRIMARY];
> + for_each_pipe(display, pipe_type) {
> +igt_pipe_t *pipe = >pipes[pipe_type];
> + igt_plane_t *plane = igt_pipe_get_plane_type(pipe,
> DRM_PLANE_TYPE_PRIMARY);
>  
>   if (plane->fb)
>   igt_plane_set_fb(plane, NULL);
-- 
Mika Kahola - Intel OTC

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

[Intel-gfx] [REGRESSION] Black screen after switching desktop session (was: Re: Linux 4.10-rc5)

[Martin Steigerwald]

Am Sonntag, 22. Januar 2017, 13:32:08 CET schrieb Linus Torvalds:
> Things seem to be calming down a bit, and everything looks nominal.
> 
> There's only been about 250 changes (not counting merges) in the last
> week, and the diffstat touches less than 300 files (with drivers and
> architecture updates being the bulk, but there's tooling, networking
> and filesystems in there too).
> 
> So keep testing, and I think we'll have a regular release schedule.

Testing this is no fun:

Bug 99533 - black screen after switching session
https://bugs.freedesktop.org/99533


This after GPU hang/lockups with Kernel 4.9 reported as for example:

Bug 98922 - [snb] GPU hang on PlaneShift
https://bugs.freedesktop.org/98922

Which may be a duplicate of #98747, #98794, #98860, #98891, #98288.


I am back at kernel 4.8.15 as I need this machine for production work.

Sometimes I wish for a microkernel that might be able to reincarnate drivers 
that hang or do wierd things like that. That may at least give a way to 
actually do some debugging or even get the desktop session back without 
loosing its state. Especially for graphics drivers and hibernating/resuming 
from hibernations which also occasionally fails – again without leaving a way 
to interact with the machine to do further debugging. Linux kernel usually 
just crashes completely, not even a ping or ssh possible, or it at least stuck 
with a black display without any way to restart the graphics driver cause it 
seems to be in some undefined state. Combined with occasionally happening bugs 
this makes triaging bugs time consuming and risky. I do like to help testing, 
but maybe its time to just switch to distro kernels and be done about it, as I 
regularily come across bugs that are too expensive for me to triage.

Please understand that I am not willing to bisect these occasionally happening 
bugs with have the potential to cause data loss due to having to switch off 
the machine forcefully. Fortunately at least KMail saves a mail I write from 
time to time and also Kate does swap files.

I am also a bit unwilling to do further debugging of this one as I usually use 
two sessions when I am at work and I risk loosing data I work on. But… at 
least with this issue it seems I would have a way to SSH into the machine 
before kicking it.


I am dissatisfied with the state of the Intel graphics driver on this ThinkPad 
T520 with Sandybridge since kernel 4.9 and wonder whether you guys at Intel 
really test things with older hardware versions.

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

Re: [Intel-gfx] [PATCH v2 02/38] drm/i915: Provide a hook for selftests

[Joonas Lahtinen]

On to, 2017-01-19 at 11:41 +, Chris Wilson wrote:
> Some pieces of code are independent of hardware but are very tricky to
> exercise through the normal userspace ABI or via debugfs hooks. Being
> able to create mock unit tests and execute them through CI is vital.
> Start by adding a central point where we can execute unit tests and
> a parameter to enable them. This is disabled by default as the
> expectation is that these tests will occasionally explode.
> 
> To facilitate integration with igt, any parameter beginning with
> i915.igt__ is interpreted as a subtest executable independently via
> igt/drv_selftest.
> 
> Two classes of selftests are recognised: mock unit tests and integration
> tests. Mock unit tests are run as soon as the module is loaded, before
> the device is probed. At that point there is no driver instantiated and
> all hw interactions must be "mocked". This is very useful for writing
> universal tests to exercise code not typically run on a broad range of
> architectures. Alternatively, you can hook into the live selftests and
> run when the device has been instantiated - hw interactions are real.
> 
> v2: Add a macro for compiling conditional code for mock objects inside
> real objects.
> v3: Differentiate between mock unit tests and late integration test.
> v4: List the tests in natural order, use igt to sort after modparam.
> v5: s/late/live/
> v6: s/unsigned long/unsigned int/
> v7: Use igt_ prefixes for long helpers.
> 
> Signed-off-by: Chris Wilson 
> Reviewed-by: Tvrtko Ursulin  #v1



> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -3,6 +3,7 @@
>  # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
>  
>  subdir-ccflags-$(CONFIG_DRM_I915_WERROR) := -Werror
> +subdir-ccflags-$(CONFIG_DRM_I915_SELFTEST) += -I$(src) -I$(src)/selftests

Similar to drm, add selftests/Makefile, to get rid of this.

> @@ -116,6 +117,9 @@ i915-y += dvo_ch7017.o \
>  
>  # Post-mortem debug and GPU hang state capture
>  i915-$(CONFIG_DRM_I915_CAPTURE_ERROR) += i915_gpu_error.o
> +i915-$(CONFIG_DRM_I915_SELFTEST) += \
> + selftests/i915_random.o \
> + selftests/i915_selftest.o
> 

Ditto.

> @@ -499,7 +501,17 @@ static int i915_pci_probe(struct pci_dev *pdev, const 
> struct pci_device_id *ent)
>   if (vga_switcheroo_client_probe_defer(pdev))
>   return -EPROBE_DEFER;
>  
> - return i915_driver_load(pdev, ent);
> + err = i915_driver_load(pdev, ent);
> + if (err)
> + return err;
> +
> + err = i915_live_selftests(pdev);
> + if (err) {
> + i915_driver_unload(pci_get_drvdata(pdev));
> + return err > 0 ? -ENOTTY : err;

What's up with this?
 
>  static void i915_pci_remove(struct pci_dev *pdev)
> @@ -521,6 +533,11 @@ static struct pci_driver i915_pci_driver = {
>  static int __init i915_init(void)
>  {
>   bool use_kms = true;
> + int err;
> +
> + err = i915_mock_selftests();
> + if (err)
> + return err > 0 ? 0 : err;

Especially this, is this for skipping the device init completely?

> +int i915_subtests(const char *caller,
> +   const struct i915_subtest *st,
> +   unsigned int count,
> +   void *data);
> +#define i915_subtests(T, data) \
> + (i915_subtests)(__func__, T, ARRAY_SIZE(T), data)

Argh, why not __i915_subtests like good people do?

> +/* Using the i915_selftest_ prefix becomes a little unwieldy with the 
> helpers.
> + * Instead we use the igt_ shorthand, in reference to the intel-gpu-tools
> + * suite of uabi test cases (which includes a test runner for our selftests).
> + */

I'd ask for an ack from Daniel/Jani on this.

> +static inline u32 i915_prandom_u32_max_state(u32 ep_ro, struct rnd_state 
> *state)
> +{
> > +   return upper_32_bits((u64)prandom_u32_state(state) * ep_ro);
> +}

Upstream material. Also I remember this stuff is in DRM too, so I
assume you cleanly copy pasted, and skip this randomization code.

> +++ b/drivers/gpu/drm/i915/selftests/i915_selftest.c



> +/* Embed the line number into the parameter name so that we can order tests 
> */
> +#define selftest(n, func) selftest_0(n, func, param(n))
> +#define param(n) __PASTE(igt__, __PASTE(__PASTE(__LINE__, __), mock_##n))

Hmm, you could reduce one __PASTE by making the ending __mock_##n?

> +static int run_selftests(const char *name,
> +  struct selftest *st,
> +  unsigned int count,
> +  void *data)
> +{
> + int err = 0;
> +
> + while (!i915_selftest.random_seed)
> + i915_selftest.random_seed = get_random_int();

You know that in theory this might take an eternity. I'm not sure why
zero is not OK after this point?

> +
> + i915_selftest.timeout_jiffies =
> + i915_selftest.timeout_ms ?
> + msecs_to_jiffies_timeout(i915_selftest.timeout_ms) :
> + MAX_SCHEDULE_TIMEOUT;

You had a default value for 

Re: [Intel-gfx] [PATCH] x86/gpu: GLK uses the same GMS values as SKL

[Ingo Molnar]

* Jani Nikula  wrote:

> On Wed, 25 Jan 2017, Jani Nikula  wrote:
> > On Wed, 25 Jan 2017, Ingo Molnar  wrote:
> >> * Paulo Zanoni  wrote:
> >>
> >>> So don't forget to reserve its stolen memory bits.
> >>> 
> >>> Cc: Ingo Molnar 
> >>> Cc: H. Peter Anvin 
> >>> Cc: Ander Conselvan de Oliveira 
> >>> Cc: x...@kernel.org
> >>> Reviewed-by: Ander Conselvan de Oliveira 
> >>> 
> >>> Signed-off-by: Paulo Zanoni 
> >>> ---
> >>>  arch/x86/kernel/early-quirks.c | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>> 
> >>> Found by code inspection. This is completely untested since I don't have
> >>> GLK hardware.
> >>> 
> >>> diff --git a/arch/x86/kernel/early-quirks.c 
> >>> b/arch/x86/kernel/early-quirks.c
> >>> index 6a08e25..23c4f1c 100644
> >>> --- a/arch/x86/kernel/early-quirks.c
> >>> +++ b/arch/x86/kernel/early-quirks.c
> >>> @@ -526,6 +526,7 @@ static const struct pci_device_id intel_early_ids[] 
> >>> __initconst = {
> >>>   INTEL_SKL_IDS(_early_ops),
> >>>   INTEL_BXT_IDS(_early_ops),
> >>>   INTEL_KBL_IDS(_early_ops),
> >>> + INTEL_GLK_IDS(_early_ops),
> >>>  };
> >>
> >> There's no INTEL_GLK_IDS() upstream - is there any dependency here on 
> >> other 
> >> changes to the i915 GPU driver?
> >
> > It's in our -next. Ack for merging this through drm-intel along with the
> > deps?

Yeah, LGTM:

  Acked-by: Ingo Molnar 

Thanks,

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

Re: [Intel-gfx] [PATCH i-g-t v2 07/33] tests/kms_chv_cursor_fail: Add support for dynamic number of planes

[Mika Kahola]

Reviewed-by: Mika Kahola 
 
On Tue, 2017-01-24 at 18:33 -0500, Robert Foss wrote:
> Add changes reflecting the new support for dynamic number of planes
> per pipe.
> 
> Signed-off-by: Robert Foss 
> ---
>  tests/kms_chv_cursor_fail.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/kms_chv_cursor_fail.c
> b/tests/kms_chv_cursor_fail.c
> index 0158580c..ce6e8df6 100644
> --- a/tests/kms_chv_cursor_fail.c
> +++ b/tests/kms_chv_cursor_fail.c
> @@ -65,7 +65,7 @@ static void cursor_disable(data_t *data)
>   igt_output_t *output = data->output;
>   igt_plane_t *cursor;
>  
> - cursor = igt_output_get_plane(output, IGT_PLANE_CURSOR);
> + cursor = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_CURSOR);
>   igt_plane_set_fb(cursor, NULL);
>  }
>  
> @@ -242,7 +242,7 @@ static void prepare_crtc(data_t *data)
>     LOCAL_DRM_FORMAT_MOD_NONE,
>     >primary_fb);
>  
> - primary = igt_output_get_plane(data->output,
> IGT_PLANE_PRIMARY);
> + primary = igt_output_get_plane_type(data->output,
> DRM_PLANE_TYPE_PRIMARY);
>   igt_plane_set_fb(primary, >primary_fb);
>  
>   igt_display_commit(display);
> @@ -277,7 +277,7 @@ static void cleanup_crtc(data_t *data)
>  
>   igt_remove_fb(data->drm_fd, >primary_fb);
>  
> - primary = igt_output_get_plane(data->output,
> IGT_PLANE_PRIMARY);
> + primary = igt_output_get_plane_type(data->output,
> DRM_PLANE_TYPE_PRIMARY);
>   igt_plane_set_fb(primary, NULL);
>  
>   igt_output_set_pipe(data->output, PIPE_ANY);
-- 
Mika Kahola - Intel OTC

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

Re: [Intel-gfx] [PATCH i-g-t v2 08/33] tests/kms_crtc_background_color: Add support for dynamic number of planes

[Mika Kahola]

Reviewed-by: Mika Kahola 

On Tue, 2017-01-24 at 18:33 -0500, Robert Foss wrote:
> Add changes reflecting the new support for dynamic number of planes
> per pipe.
> 
> Signed-off-by: Robert Foss 
> ---
>  tests/kms_crtc_background_color.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/kms_crtc_background_color.c
> b/tests/kms_crtc_background_color.c
> index 537d4ce6..d6dd8d90 100644
> --- a/tests/kms_crtc_background_color.c
> +++ b/tests/kms_crtc_background_color.c
> @@ -136,7 +136,7 @@ static void test_crtc_background(data_t *data)
>  
>   igt_output_set_pipe(output, pipe);
>  
> - plane = igt_output_get_plane(output,
> IGT_PLANE_PRIMARY);
> + plane = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
>   igt_require(plane->pipe->background_property);
>  
>   prepare_crtc(data, output, pipe, plane, 1, PURPLE,
> BLACK64);
-- 
Mika Kahola - Intel OTC

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

Re: [Intel-gfx] [PATCH i-g-t v2 09/33] tests/kms_cursor_crc: Add support for dynamic number of planes

[Mika Kahola]

Reviewed-by: Mika Kahola 

On Tue, 2017-01-24 at 18:33 -0500, Robert Foss wrote:
> Add changes reflecting the new support for dynamic number of planes
> per pipe.
> 
> Signed-off-by: Robert Foss 
> ---
>  tests/kms_cursor_crc.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> index ff931607..4851e18f 100644
> --- a/tests/kms_cursor_crc.c
> +++ b/tests/kms_cursor_crc.c
> @@ -92,7 +92,7 @@ static void cursor_enable(data_t *data)
>   igt_output_t *output = data->output;
>   igt_plane_t *cursor;
>  
> - cursor = igt_output_get_plane(output, IGT_PLANE_CURSOR);
> + cursor = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_CURSOR);
>   igt_plane_set_fb(cursor, >fb);
>   igt_plane_set_size(cursor, data->curw, data->curh);
>  }
> @@ -102,7 +102,7 @@ static void cursor_disable(data_t *data)
>   igt_output_t *output = data->output;
>   igt_plane_t *cursor;
>  
> - cursor = igt_output_get_plane(output, IGT_PLANE_CURSOR);
> + cursor = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_CURSOR);
>   igt_plane_set_fb(cursor, NULL);
>  }
>  
> @@ -120,7 +120,7 @@ static void do_single_test(data_t *data, int x,
> int y)
>   /* Hardware test */
>   igt_paint_test_pattern(cr, data->screenw, data->screenh);
>   cursor_enable(data);
> - cursor = igt_output_get_plane(data->output,
> IGT_PLANE_CURSOR);
> + cursor = igt_output_get_plane_type(data->output,
> DRM_PLANE_TYPE_CURSOR);
>   igt_plane_set_position(cursor, x, y);
>   igt_display_commit(display);
>   igt_wait_for_vblank(data->drm_fd, data->pipe);
> @@ -174,7 +174,7 @@ static void do_fail_test(data_t *data, int x, int
> y, int expect)
>   /* Hardware test */
>   igt_paint_test_pattern(cr, data->screenw, data->screenh);
>   cursor_enable(data);
> - cursor = igt_output_get_plane(data->output,
> IGT_PLANE_CURSOR);
> + cursor = igt_output_get_plane_type(data->output,
> DRM_PLANE_TYPE_CURSOR);
>   igt_plane_set_position(cursor, x, y);
>   ret = igt_display_try_commit2(display, COMMIT_LEGACY);
>  
> @@ -301,7 +301,7 @@ static void prepare_crtc(data_t *data,
> igt_output_t *output,
>   0.0, 0.0, 0.0,
>   >primary_fb);
>  
> - primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
> + primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
>   igt_plane_set_fb(primary, >primary_fb);
>  
>   igt_display_commit(display);
> @@ -342,7 +342,7 @@ static void cleanup_crtc(data_t *data,
> igt_output_t *output)
>  
>   igt_remove_fb(data->drm_fd, >primary_fb);
>  
> - primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
> + primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
>   igt_plane_set_fb(primary, NULL);
>  
>   igt_output_set_pipe(output, PIPE_ANY);
-- 
Mika Kahola - Intel OTC

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