Re: [Intel-gfx] linux-next: build failure after merge of the drm tree

[Sedat Dilek]

On 7/15/16, Stephen Rothwell  wrote:
> Hi Dave,
>
> After merging the drm tree, today's linux-next build (x86_64 allmodconfig)
> failed like this:
>
> In file included from drivers/gpu/drm/i915/intel_opregion.c:34:0:
> drivers/gpu/drm/i915/intel_opregion.c: In function
> 'intel_opregion_get_panel_type':
> drivers/gpu/drm/i915/intel_opregion.c:1042:17: error: 'dev' undeclared
> (first use in this function)
>   if (IS_SKYLAKE(dev)) {
>  ^
> drivers/gpu/drm/i915/i915_drv.h:2603:43: note: in definition of macro
> '__I915__'
>   if (__builtin_types_compatible_p(typeof(*p), struct drm_i915_private)) \
>^
> drivers/gpu/drm/i915/i915_drv.h:2670:26: note: in expansion of macro
> 'INTEL_INFO'
>  #define IS_SKYLAKE(dev) (INTEL_INFO(dev)->is_skylake)
>   ^
> drivers/gpu/drm/i915/intel_opregion.c:1042:6: note: in expansion of macro
> 'IS_SKYLAKE'
>   if (IS_SKYLAKE(dev)) {
>   ^
> drivers/gpu/drm/i915/intel_opregion.c:1042:17: note: each undeclared
> identifier is reported only once for each function it appears in
>   if (IS_SKYLAKE(dev)) {
>  ^
> drivers/gpu/drm/i915/i915_drv.h:2603:43: note: in definition of macro
> '__I915__'
>   if (__builtin_types_compatible_p(typeof(*p), struct drm_i915_private)) \
>^
> drivers/gpu/drm/i915/i915_drv.h:2670:26: note: in expansion of macro
> 'INTEL_INFO'
>  #define IS_SKYLAKE(dev) (INTEL_INFO(dev)->is_skylake)
>   ^
> drivers/gpu/drm/i915/intel_opregion.c:1042:6: note: in expansion of macro
> 'IS_SKYLAKE'
>   if (IS_SKYLAKE(dev)) {
>   ^
>
> Caused by commit
>
>   6f9f4b7a2cf7 ("drm/i915/opregion: Convert to using native
> drm_i915_private")
>
> interacting with commit
>
>   aeddda06c1a7 ("drm/i915: Ignore panel type from OpRegion on SKL")
>
> from the drm-intel-fixes tree.
>
> I applied this merge fix patch:
>
> From: Stephen Rothwell 
> Date: Fri, 15 Jul 2016 13:34:05 +1000
> Subject: [PATCH] drm/i915/opregion: fix up for argument change
>
> Signed-off-by: Stephen Rothwell 
> ---
>  drivers/gpu/drm/i915/intel_opregion.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c
> b/drivers/gpu/drm/i915/intel_opregion.c
> index 21f19641e33e..dcdb346b596c 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -1039,7 +1039,7 @@ intel_opregion_get_panel_type(struct drm_i915_private
> *dev_priv)
>* vswing instead. Low vswing results in some display flickers, so
>* let's simply ignore the OpRegion panel type on SKL for now.
>*/
> - if (IS_SKYLAKE(dev)) {
> + if (IS_SKYLAKE(dev_priv)) {
>   DRM_DEBUG_KMS("Ignoring OpRegion panel type (%d)\n", ret - 1);
>   return -ENODEV;
>   }
> --
> 2.8.1
>

I fell over the same issue when testing drm-intel-nightly and sent a patch.

- Sedat -

https://lists.freedesktop.org/archives/intel-gfx/2016-July/100691.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] linux-next: build failure after merge of the drm tree

[Stephen Rothwell]

Hi Dave,

After merging the drm tree, today's linux-next build (x86_64 allmodconfig)
failed like this:

In file included from drivers/gpu/drm/i915/intel_opregion.c:34:0:
drivers/gpu/drm/i915/intel_opregion.c: In function 
'intel_opregion_get_panel_type':
drivers/gpu/drm/i915/intel_opregion.c:1042:17: error: 'dev' undeclared (first 
use in this function)
  if (IS_SKYLAKE(dev)) {
 ^
drivers/gpu/drm/i915/i915_drv.h:2603:43: note: in definition of macro '__I915__'
  if (__builtin_types_compatible_p(typeof(*p), struct drm_i915_private)) \
   ^
drivers/gpu/drm/i915/i915_drv.h:2670:26: note: in expansion of macro 
'INTEL_INFO'
 #define IS_SKYLAKE(dev) (INTEL_INFO(dev)->is_skylake)
  ^
drivers/gpu/drm/i915/intel_opregion.c:1042:6: note: in expansion of macro 
'IS_SKYLAKE'
  if (IS_SKYLAKE(dev)) {
  ^
drivers/gpu/drm/i915/intel_opregion.c:1042:17: note: each undeclared identifier 
is reported only once for each function it appears in
  if (IS_SKYLAKE(dev)) {
 ^
drivers/gpu/drm/i915/i915_drv.h:2603:43: note: in definition of macro '__I915__'
  if (__builtin_types_compatible_p(typeof(*p), struct drm_i915_private)) \
   ^
drivers/gpu/drm/i915/i915_drv.h:2670:26: note: in expansion of macro 
'INTEL_INFO'
 #define IS_SKYLAKE(dev) (INTEL_INFO(dev)->is_skylake)
  ^
drivers/gpu/drm/i915/intel_opregion.c:1042:6: note: in expansion of macro 
'IS_SKYLAKE'
  if (IS_SKYLAKE(dev)) {
  ^

Caused by commit

  6f9f4b7a2cf7 ("drm/i915/opregion: Convert to using native drm_i915_private")

interacting with commit

  aeddda06c1a7 ("drm/i915: Ignore panel type from OpRegion on SKL")

from the drm-intel-fixes tree.

I applied this merge fix patch:

From: Stephen Rothwell 
Date: Fri, 15 Jul 2016 13:34:05 +1000
Subject: [PATCH] drm/i915/opregion: fix up for argument change

Signed-off-by: Stephen Rothwell 
---
 drivers/gpu/drm/i915/intel_opregion.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
b/drivers/gpu/drm/i915/intel_opregion.c
index 21f19641e33e..dcdb346b596c 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -1039,7 +1039,7 @@ intel_opregion_get_panel_type(struct drm_i915_private 
*dev_priv)
 * vswing instead. Low vswing results in some display flickers, so
 * let's simply ignore the OpRegion panel type on SKL for now.
 */
-   if (IS_SKYLAKE(dev)) {
+   if (IS_SKYLAKE(dev_priv)) {
DRM_DEBUG_KMS("Ignoring OpRegion panel type (%d)\n", ret - 1);
return -ENODEV;
}
-- 
2.8.1

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

Re: [Intel-gfx] [PATCH] drm/i915: Acquire intel_runtime_pm for HD-Audio registers

[Yang, Libin]

Hi Wilson,

> -Original Message-
> From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> Sent: Thursday, July 14, 2016 10:34 PM
> To: Mika Kuoppala ; intel-
> g...@lists.freedesktop.org; Takashi Iwai ; Yang, Libin
> 
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Acquire intel_runtime_pm for HD-
> Audio registers
> 
> On Wed, Jul 13, 2016 at 05:01:02PM +0300, Marius Vlad wrote:
> > Did try when you submitted the patch...but can't seem to replicate
> > with latest nightly on other SKLs, and currently do not have access on
> > the machine that caused it.
> 
> So fwiw, Hans de Geode confirmed that only reverting 03b135cebc47 (which
> enables the display powerwell as well) was enough.
> 
> Libin, do you want to handle the revert or explain the mistake in

What's the problem with the patch? I can find the details from the email thread.

HSW/BSW need acquire the power because it is inside the gpu. So we 
limited need i915 power for controller to HSW/BDW. 

> 
> commit 03b135cebc47d75ea2dc346770374ab741966955
> Author: Libin Yang 
> Date:   Wed Jun 3 09:30:15 2015 +0800
> 
> ALSA: hda - remove controller dependency on i915 power well for SKL
> 
> -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

[Intel-gfx] [PATCH d-i-n] drm/i915: Fix build failure in intel_opregion_get_panel_type()

[Sedat Dilek]

Tested against drm-intel-nightly (2016y-07m-14d-20h-15m-29s UTC).

Fixes: aeddda06c1a704bb9 ("drm/i915: Ignore panel type from OpRegion on SKL")
CC: intel-gfx@lists.freedesktop.org
CC: Ville Syrjälä 
CC: Daniel Vetter 
Signed-off-by: Sedat Dilek 
---
 drivers/gpu/drm/i915/intel_opregion.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
b/drivers/gpu/drm/i915/intel_opregion.c
index c804630d10d2..adca262d591a 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -1078,7 +1078,7 @@ intel_opregion_get_panel_type(struct drm_i915_private 
*dev_priv)
 * vswing instead. Low vswing results in some display flickers, so
 * let's simply ignore the OpRegion panel type on SKL for now.
 */
-   if (IS_SKYLAKE(dev)) {
+   if (IS_SKYLAKE(dev_priv)) {
DRM_DEBUG_KMS("Ignoring OpRegion panel type (%d)\n", ret - 1);
return -ENODEV;
}
-- 
2.9.0

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

Re: [Intel-gfx] [PATCH] Bump libdrm-intel dependency library to a newer version that support softpin.

[Chris Wilson]

On Thu, Jul 14, 2016 at 09:52:24PM +0100, Emil Velikov wrote:
> On 14 July 2016 at 21:03, Chris Wilson  wrote:
> > On Thu, Jul 14, 2016 at 07:54:59PM +0100, Emil Velikov wrote:
> >> On 14 July 2016 at 17:49, Chris Wilson  wrote:
> >> > On Thu, Jul 14, 2016 at 05:29:55PM +0100, Emil Velikov wrote:
> >> >> Hi Marius,
> >> >>
> >> >> Just double-checking - this is an i-g-t patch, isn't it ?
> >> >>
> >> >> On 14 July 2016 at 11:39, Marius Vlad  wrote:
> >> >> > Required by commit 2603b98ca (aubdump: Support softpin bos).
> >> >> >
> >> >> > Signed-off-by: Marius Vlad 
> >> >> > CC: Kristian Høgsberg Kristensen 
> >> >> > ---
> >> >> >  configure.ac | 2 +-
> >> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >> >
> >> >> > diff --git a/configure.ac b/configure.ac
> >> >> > index f05bcb0..ade9756 100644
> >> >> > --- a/configure.ac
> >> >> > +++ b/configure.ac
> >> >> > @@ -119,7 +119,7 @@ if test "x$GCC" = "xyes"; then
> >> >> >  fi
> >> >> >  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
> >> >> >
> >> >> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm])
> >> >> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.68 libdrm])
> >> >> Yes please. As you do that one can nuke most/all the "define LOCAL_"
> >> >> and "struct local_" (in lib/ioctl_wrappers.h)
> >> >> and replace with with the official symbols. A very nice plus imho ;-)
> >> >
> >> > Please don't. It makes running on older installations even more
> >> > cumbersome.
> >> Slightly confused here: are you against the libdrm_intel bump, or the
> >> removal of the local symbols ?
> >
> > Local symbols. They save a lot of time if you can just get the test you
> > need compiling and not worry about dependencies. One of the basic
> > tenents is that we drop a new kernel into an old userspace and expect to
> > have not broken anything. Being lazy, for smoke testing I just build
> > in situ.
> >
> Fully agree.
> 
> >> Admittedly sometimes distros don't bother/refuse to update libdrm
> >> which could be an issue in the former case. Although if the package
> >> (with all the definitions) is compulsory, how would that cause an
> >> issue ?
> >
> > The package may not even exist when testing on v.old distro images.
> > It is mostly a major of convenience, but since the work is already done
> > to be independent, removing causes more work.
> Seems that things are not completely independent, as illustrated by
> the version bump.
>
> Thus was wondering what's the benefit of (or why object against
> nuking) the local copies. Since even if they were needed before, they
> won't be after the bump.That said, it seems that it's a picky/sore
> topic so I won't dig into it.

I don't have uptodate or even recent libdrm everywhere. Getting rid of
the locals means another dependency chain to build.

Just being completely lazy and saying "if ain't broke, don't try to fix it".
-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] Bump libdrm-intel dependency library to a newer version that support softpin.

[Emil Velikov]

On 14 July 2016 at 21:03, Chris Wilson  wrote:
> On Thu, Jul 14, 2016 at 07:54:59PM +0100, Emil Velikov wrote:
>> On 14 July 2016 at 17:49, Chris Wilson  wrote:
>> > On Thu, Jul 14, 2016 at 05:29:55PM +0100, Emil Velikov wrote:
>> >> Hi Marius,
>> >>
>> >> Just double-checking - this is an i-g-t patch, isn't it ?
>> >>
>> >> On 14 July 2016 at 11:39, Marius Vlad  wrote:
>> >> > Required by commit 2603b98ca (aubdump: Support softpin bos).
>> >> >
>> >> > Signed-off-by: Marius Vlad 
>> >> > CC: Kristian Høgsberg Kristensen 
>> >> > ---
>> >> >  configure.ac | 2 +-
>> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/configure.ac b/configure.ac
>> >> > index f05bcb0..ade9756 100644
>> >> > --- a/configure.ac
>> >> > +++ b/configure.ac
>> >> > @@ -119,7 +119,7 @@ if test "x$GCC" = "xyes"; then
>> >> >  fi
>> >> >  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
>> >> >
>> >> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm])
>> >> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.68 libdrm])
>> >> Yes please. As you do that one can nuke most/all the "define LOCAL_"
>> >> and "struct local_" (in lib/ioctl_wrappers.h)
>> >> and replace with with the official symbols. A very nice plus imho ;-)
>> >
>> > Please don't. It makes running on older installations even more
>> > cumbersome.
>> Slightly confused here: are you against the libdrm_intel bump, or the
>> removal of the local symbols ?
>
> Local symbols. They save a lot of time if you can just get the test you
> need compiling and not worry about dependencies. One of the basic
> tenents is that we drop a new kernel into an old userspace and expect to
> have not broken anything. Being lazy, for smoke testing I just build
> in situ.
>
Fully agree.

>> Admittedly sometimes distros don't bother/refuse to update libdrm
>> which could be an issue in the former case. Although if the package
>> (with all the definitions) is compulsory, how would that cause an
>> issue ?
>
> The package may not even exist when testing on v.old distro images.
> It is mostly a major of convenience, but since the work is already done
> to be independent, removing causes more work.
Seems that things are not completely independent, as illustrated by
the version bump.

Thus was wondering what's the benefit of (or why object against
nuking) the local copies. Since even if they were needed before, they
won't be after the bump.That said, it seems that it's a picky/sore
topic so I won't dig into it.

If anything I'll just send patches to be acked/nacked.
-Emil
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Ro.CI.BAT: warning for Fixes for HPD (rev3)

[Daniel Vetter]

On Wed, Jun 22, 2016 at 01:22:09PM -, Patchwork wrote:
> == Series Details ==
> 
> Series: Fixes for HPD (rev3)
> URL   : https://patchwork.freedesktop.org/series/8870/
> State : warning
> 
> == Summary ==
> 
> Series 8870v3 Fixes for HPD
> http://patchwork.freedesktop.org/api/1.0/series/8870/revisions/3/mbox
> 
> Test kms_pipe_crc_basic:
> Subgroup suspend-read-crc-pipe-a:
> skip   -> DMESG-WARN (ro-bdw-i7-5557U)
> skip   -> DMESG-WARN (ro-bdw-i5-5250u)
> Subgroup suspend-read-crc-pipe-c:
> skip   -> DMESG-WARN (ro-bdw-i7-5557U)
> skip   -> DMESG-WARN (ro-bdw-i5-5250u)

Say hello to the bdw link training fail that's happening all over
recently:

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

Since it's not entirely clear (at least to me) whether the "enabling power
well causes hpd" issue is still present that Ville reported, and since
these patches seem stuck forever, I went ahead and applied. Lyude can't
repro, and afaik Ville said that this spurious hpd is rather sporadic.
-Daniel

> 
> fi-skl-i7-6700k  total:226  pass:188  dwarn:0   dfail:0   fail:1   skip:37 
> ro-bdw-i5-5250u  total:226  pass:197  dwarn:4   dfail:0   fail:1   skip:24 
> ro-bdw-i7-5557U  total:226  pass:198  dwarn:3   dfail:0   fail:1   skip:24 
> ro-bdw-i7-5600u  total:226  pass:185  dwarn:0   dfail:0   fail:1   skip:40 
> ro-bsw-n3050 total:226  pass:170  dwarn:1   dfail:0   fail:4   skip:51 
> ro-byt-n2820 total:226  pass:173  dwarn:0   dfail:0   fail:4   skip:49 
> ro-hsw-i3-4010u  total:226  pass:190  dwarn:0   dfail:0   fail:1   skip:35 
> ro-hsw-i7-4770r  total:226  pass:190  dwarn:0   dfail:0   fail:1   skip:35 
> ro-ilk-i7-620lm  total:226  pass:150  dwarn:0   dfail:0   fail:2   skip:74 
> ro-ilk1-i5-650   total:221  pass:150  dwarn:0   dfail:0   fail:2   skip:69 
> ro-ivb-i7-3770   total:226  pass:181  dwarn:0   dfail:0   fail:1   skip:44 
> ro-ivb2-i7-3770  total:226  pass:185  dwarn:0   dfail:0   fail:1   skip:40 
> ro-skl3-i5-6260u total:226  pass:201  dwarn:1   dfail:0   fail:1   skip:23 
> ro-snb-i7-2620M  total:226  pass:174  dwarn:0   dfail:0   fail:2   skip:50 
> fi-hsw-i7-4770k failed to connect after reboot
> fi-kbl-qkkr failed to connect after reboot
> fi-skl-i5-6260u failed to connect after reboot
> fi-snb-i7-2600 failed to connect after reboot
> 
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1267/
> 
> f1f5a05 drm-intel-nightly: 2016y-06m-22d-10h-45m-32s UTC integration manifest
> 4f8a723 drm/i915: Enable polling when we don't have hpd
> 690a832 drm/i915/vlv: Disable HPD in valleyview_crt_detect_hotplug()
> 990a134 drm/i915/vlv: Reset the ADPA in vlv_display_power_well_init()
> ae95081 drm/i915/vlv: Make intel_crt_reset() per-encoder
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

[Daniel Vetter]

Hi Dave,

Just 2 regression fixes.

I've also realized that a pile of hang fixes for kbl landed in next, and
no one thought of backporting it to 4.7 - kbl has lost prelim_hw_support
tagging in 4.7-rc1 already. Mika is prepping a topic branch for those,
will send you a separate pull request since it's quite a bit (but should
be all well restricted to kbl code, so similar to polaris in amdgpu).

Cheers, Daniel


The following changes since commit cab103274352721b77fc5923a631fc63350bef8e:

  drm/i915: Fix missing unlock on error in i915_ppgtt_info() (2016-06-30 
13:30:15 +0300)

are available in the git repository at:

  git://anongit.freedesktop.org/drm-intel tags/drm-intel-fixes-2016-07-14

for you to fetch changes up to aeddda06c1a704bb97c8a7bfe7a472120193bd56:

  drm/i915: Ignore panel type from OpRegion on SKL (2016-07-14 16:08:04 +0200)


Chris Wilson (1):
  drm/i915: Update ifdeffery for mutex->owner

Ville Syrjälä (1):
  drm/i915: Ignore panel type from OpRegion on SKL

 drivers/gpu/drm/i915/i915_gem_shrinker.c |  2 +-
 drivers/gpu/drm/i915/intel_opregion.c| 11 +++
 2 files changed, 12 insertions(+), 1 deletion(-)

-- 
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] Bump libdrm-intel dependency library to a newer version that support softpin.

[Chris Wilson]

On Thu, Jul 14, 2016 at 07:54:59PM +0100, Emil Velikov wrote:
> On 14 July 2016 at 17:49, Chris Wilson  wrote:
> > On Thu, Jul 14, 2016 at 05:29:55PM +0100, Emil Velikov wrote:
> >> Hi Marius,
> >>
> >> Just double-checking - this is an i-g-t patch, isn't it ?
> >>
> >> On 14 July 2016 at 11:39, Marius Vlad  wrote:
> >> > Required by commit 2603b98ca (aubdump: Support softpin bos).
> >> >
> >> > Signed-off-by: Marius Vlad 
> >> > CC: Kristian Høgsberg Kristensen 
> >> > ---
> >> >  configure.ac | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/configure.ac b/configure.ac
> >> > index f05bcb0..ade9756 100644
> >> > --- a/configure.ac
> >> > +++ b/configure.ac
> >> > @@ -119,7 +119,7 @@ if test "x$GCC" = "xyes"; then
> >> >  fi
> >> >  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
> >> >
> >> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm])
> >> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.68 libdrm])
> >> Yes please. As you do that one can nuke most/all the "define LOCAL_"
> >> and "struct local_" (in lib/ioctl_wrappers.h)
> >> and replace with with the official symbols. A very nice plus imho ;-)
> >
> > Please don't. It makes running on older installations even more
> > cumbersome.
> Slightly confused here: are you against the libdrm_intel bump, or the
> removal of the local symbols ?

Local symbols. They save a lot of time if you can just get the test you
need compiling and not worry about dependencies. One of the basic
tenents is that we drop a new kernel into an old userspace and expect to
have not broken anything. Being lazy, for smoke testing I just build
in situ.

> Admittedly sometimes distros don't bother/refuse to update libdrm
> which could be an issue in the former case. Although if the package
> (with all the definitions) is compulsory, how would that cause an
> issue ?

The package may not even exist when testing on v.old distro images.
It is mostly a major of convenience, but since the work is already done
to be independent, removing causes more work.
-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] Bump libdrm-intel dependency library to a newer version that support softpin.

[Emil Velikov]

On 14 July 2016 at 17:49, Chris Wilson  wrote:
> On Thu, Jul 14, 2016 at 05:29:55PM +0100, Emil Velikov wrote:
>> Hi Marius,
>>
>> Just double-checking - this is an i-g-t patch, isn't it ?
>>
>> On 14 July 2016 at 11:39, Marius Vlad  wrote:
>> > Required by commit 2603b98ca (aubdump: Support softpin bos).
>> >
>> > Signed-off-by: Marius Vlad 
>> > CC: Kristian Høgsberg Kristensen 
>> > ---
>> >  configure.ac | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/configure.ac b/configure.ac
>> > index f05bcb0..ade9756 100644
>> > --- a/configure.ac
>> > +++ b/configure.ac
>> > @@ -119,7 +119,7 @@ if test "x$GCC" = "xyes"; then
>> >  fi
>> >  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
>> >
>> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm])
>> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.68 libdrm])
>> Yes please. As you do that one can nuke most/all the "define LOCAL_"
>> and "struct local_" (in lib/ioctl_wrappers.h)
>> and replace with with the official symbols. A very nice plus imho ;-)
>
> Please don't. It makes running on older installations even more
> cumbersome.
Slightly confused here: are you against the libdrm_intel bump, or the
removal of the local symbols ?

Admittedly sometimes distros don't bother/refuse to update libdrm
which could be an issue in the former case. Although if the package
(with all the definitions) is compulsory, how would that cause an
issue ?

Genuine question here, not trying to be smart/cheeky/etc.

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

[Intel-gfx] [drm-intel:drm-intel-nightly 2/10] drivers/gpu/drm/i915/i915_drv.h:2695:26: note: in expansion of macro 'INTEL_INFO'

[kbuild test robot]

tree:   git://anongit.freedesktop.org/drm-intel drm-intel-nightly
head:   2d854c67e3af36b190e8499a3bfad7cdccde0f67
commit: 01734c300fbff01e321d4ff1b3c91e24e0a3b90d [2/10] Merge remote-tracking 
branch 'origin/drm-intel-next-fixes' into drm-intel-nightly
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
git checkout 01734c300fbff01e321d4ff1b3c91e24e0a3b90d
# save the attached .config to linux build tree
make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/i915/intel_opregion.c:34:0:
   drivers/gpu/drm/i915/intel_opregion.c: In function 
'intel_opregion_get_panel_type':
   drivers/gpu/drm/i915/intel_opregion.c:1081:17: error: 'dev' undeclared 
(first use in this function)
 if (IS_SKYLAKE(dev)) {
^
   drivers/gpu/drm/i915/i915_drv.h:2628:43: note: in definition of macro 
'__I915__'
 if (__builtin_types_compatible_p(typeof(*p), struct drm_i915_private)) \
  ^
>> drivers/gpu/drm/i915/i915_drv.h:2695:26: note: in expansion of macro 
>> 'INTEL_INFO'
#define IS_SKYLAKE(dev) (INTEL_INFO(dev)->is_skylake)
 ^~
>> drivers/gpu/drm/i915/intel_opregion.c:1081:6: note: in expansion of macro 
>> 'IS_SKYLAKE'
 if (IS_SKYLAKE(dev)) {
 ^~
   drivers/gpu/drm/i915/intel_opregion.c:1081:17: note: each undeclared 
identifier is reported only once for each function it appears in
 if (IS_SKYLAKE(dev)) {
^
   drivers/gpu/drm/i915/i915_drv.h:2628:43: note: in definition of macro 
'__I915__'
 if (__builtin_types_compatible_p(typeof(*p), struct drm_i915_private)) \
  ^
>> drivers/gpu/drm/i915/i915_drv.h:2695:26: note: in expansion of macro 
>> 'INTEL_INFO'
#define IS_SKYLAKE(dev) (INTEL_INFO(dev)->is_skylake)
 ^~
>> drivers/gpu/drm/i915/intel_opregion.c:1081:6: note: in expansion of macro 
>> 'IS_SKYLAKE'
 if (IS_SKYLAKE(dev)) {
 ^~

vim +/INTEL_INFO +2695 drivers/gpu/drm/i915/i915_drv.h

351e3db2 Brad Volkin2014-02-18  2622int count;
351e3db2 Brad Volkin2014-02-18  2623  };
351e3db2 Brad Volkin2014-02-18  2624  
dbbe9127 Chris Wilson   2014-08-09  2625  /* Note that the (struct 
drm_i915_private *) cast is just to shut up gcc. */
7312e2dd Chris Wilson   2014-08-13  2626  #define __I915__(p) ({ \
7312e2dd Chris Wilson   2014-08-13  2627struct drm_i915_private *__p; \
7312e2dd Chris Wilson   2014-08-13 @2628if 
(__builtin_types_compatible_p(typeof(*p), struct drm_i915_private)) \
7312e2dd Chris Wilson   2014-08-13  2629__p = (struct 
drm_i915_private *)p; \
7312e2dd Chris Wilson   2014-08-13  2630else if 
(__builtin_types_compatible_p(typeof(*p), struct drm_device)) \
7312e2dd Chris Wilson   2014-08-13  2631__p = to_i915((struct 
drm_device *)p); \
7312e2dd Chris Wilson   2014-08-13  2632else \
7312e2dd Chris Wilson   2014-08-13  2633BUILD_BUG(); \
7312e2dd Chris Wilson   2014-08-13  2634__p; \
7312e2dd Chris Wilson   2014-08-13  2635  })
dbbe9127 Chris Wilson   2014-08-09  2636  #define INTEL_INFO(p) 
(&__I915__(p)->info)
3f10e82f Jani Nikula2016-04-07  2637  #define INTEL_GEN(p)  
(INTEL_INFO(p)->gen)
87f1f465 Chris Wilson   2014-08-09  2638  #define INTEL_DEVID(p)
(INTEL_INFO(p)->device_id)
cae5852d Zou Nan hai2010-11-09  2639  
e87a005d Jani Nikula2015-10-20  2640  #define REVID_FOREVER 
0xff
091387c1 Chris Wilson   2016-06-24  2641  #define INTEL_REVID(p)
(__I915__(p)->drm.pdev->revision)
ac657f64 Tvrtko Ursulin 2016-05-10  2642  
ac657f64 Tvrtko Ursulin 2016-05-10  2643  #define GEN_FOREVER (0)
ac657f64 Tvrtko Ursulin 2016-05-10  2644  /*
ac657f64 Tvrtko Ursulin 2016-05-10  2645   * Returns true if Gen is in 
inclusive range [Start, End].
ac657f64 Tvrtko Ursulin 2016-05-10  2646   *
ac657f64 Tvrtko Ursulin 2016-05-10  2647   * Use GEN_FOREVER for unbound 
start and or end.
ac657f64 Tvrtko Ursulin 2016-05-10  2648   */
ac657f64 Tvrtko Ursulin 2016-05-10  2649  #define IS_GEN(p, s, e) ({ \
ac657f64 Tvrtko Ursulin 2016-05-10  2650unsigned int __s = (s), __e = 
(e); \
ac657f64 Tvrtko Ursulin 2016-05-10  2651
BUILD_BUG_ON(!__builtin_constant_p(s)); \
ac657f64 Tvrtko Ursulin 2016-05-10  2652
BUILD_BUG_ON(!__builtin_constant_p(e)); \
ac657f64 Tvrtko Ursulin 2016-05-10  2653if ((__s) != GEN_FOREVER) \
ac657f64 Tvrtko Ursulin 2016-05-10  2654__s = (s) - 1; \
ac657f64 Tvrtko Ursulin 2016-05-10  2655if ((__e) == GEN_FOREVER) \
ac657f64 Tvrtko Ursulin 2016-05-10  2656__e = BITS_PER_LONG - 
1; \
ac657f64 Tvrtko Ursulin 2016-05-10  2657else \
ac657f64 Tvrtko 

[Intel-gfx] [drm-intel:drm-intel-nightly 2/10] drivers/gpu/drm/i915/intel_opregion.c:1081:2: note: in expansion of macro 'if'

[kbuild test robot]

tree:   git://anongit.freedesktop.org/drm-intel drm-intel-nightly
head:   2d854c67e3af36b190e8499a3bfad7cdccde0f67
commit: 01734c300fbff01e321d4ff1b3c91e24e0a3b90d [2/10] Merge remote-tracking 
branch 'origin/drm-intel-next-fixes' into drm-intel-nightly
config: x86_64-randconfig-s1-07150032 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
git checkout 01734c300fbff01e321d4ff1b3c91e24e0a3b90d
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/ioport.h:12:0,
from include/linux/acpi.h:25,
from drivers/gpu/drm/i915/intel_opregion.c:28:
   drivers/gpu/drm/i915/intel_opregion.c: In function 
'intel_opregion_get_panel_type':
   drivers/gpu/drm/i915/intel_opregion.c:1081:17: error: 'dev' undeclared 
(first use in this function)
 if (IS_SKYLAKE(dev)) {
^
   include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
 if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
 ^~~~
>> drivers/gpu/drm/i915/intel_opregion.c:1081:2: note: in expansion of macro 
>> 'if'
 if (IS_SKYLAKE(dev)) {
 ^~
   include/linux/compiler.h:149:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
  ^~
>> drivers/gpu/drm/i915/i915_drv.h:2628:2: note: in expansion of macro 'if'
 if (__builtin_types_compatible_p(typeof(*p), struct drm_i915_private)) \
 ^~
>> drivers/gpu/drm/i915/i915_drv.h:2636:26: note: in expansion of macro 
>> '__I915__'
#define INTEL_INFO(p)  (&__I915__(p)->info)
 ^~~~
   drivers/gpu/drm/i915/i915_drv.h:2695:26: note: in expansion of macro 
'INTEL_INFO'
#define IS_SKYLAKE(dev) (INTEL_INFO(dev)->is_skylake)
 ^~
   drivers/gpu/drm/i915/intel_opregion.c:1081:6: note: in expansion of macro 
'IS_SKYLAKE'
 if (IS_SKYLAKE(dev)) {
 ^~
   drivers/gpu/drm/i915/intel_opregion.c:1081:17: note: each undeclared 
identifier is reported only once for each function it appears in
 if (IS_SKYLAKE(dev)) {
^
   include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
 if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
 ^~~~
>> drivers/gpu/drm/i915/intel_opregion.c:1081:2: note: in expansion of macro 
>> 'if'
 if (IS_SKYLAKE(dev)) {
 ^~
   include/linux/compiler.h:149:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
  ^~
>> drivers/gpu/drm/i915/i915_drv.h:2628:2: note: in expansion of macro 'if'
 if (__builtin_types_compatible_p(typeof(*p), struct drm_i915_private)) \
 ^~
>> drivers/gpu/drm/i915/i915_drv.h:2636:26: note: in expansion of macro 
>> '__I915__'
#define INTEL_INFO(p)  (&__I915__(p)->info)
 ^~~~
   drivers/gpu/drm/i915/i915_drv.h:2695:26: note: in expansion of macro 
'INTEL_INFO'
#define IS_SKYLAKE(dev) (INTEL_INFO(dev)->is_skylake)
 ^~
   drivers/gpu/drm/i915/intel_opregion.c:1081:6: note: in expansion of macro 
'IS_SKYLAKE'
 if (IS_SKYLAKE(dev)) {
 ^~

vim +/if +1081 drivers/gpu/drm/i915/intel_opregion.c

a0562819 Ville Syrjälä 2016-04-11  1065 DRM_DEBUG_KMS("Invalid 
OpRegion panel type 0x%x\n", ret);
a0562819 Ville Syrjälä 2016-04-11  1066 return -EINVAL;
a0562819 Ville Syrjälä 2016-04-11  1067 }
a0562819 Ville Syrjälä 2016-04-11  1068  
a0562819 Ville Syrjälä 2016-04-11  1069 /* fall back to VBT panel type? 
*/
a0562819 Ville Syrjälä 2016-04-11  1070 if (ret == 0x0) {
a0562819 Ville Syrjälä 2016-04-11  1071 DRM_DEBUG_KMS("No panel 
type in OpRegion\n");
a0562819 Ville Syrjälä 2016-04-11  1072 return -ENODEV;
a0562819 Ville Syrjälä 2016-04-11  1073 }
a0562819 Ville Syrjälä 2016-04-11  1074  
aeddda06 Ville Syrjälä 2016-07-12  1075 /*
aeddda06 Ville Syrjälä 2016-07-12  1076  * FIXME On Dell XPS 13 9350 
the OpRegion panel type (0) gives us
aeddda06 Ville Syrjälä 2016-07-12  1077  * low vswing for eDP, whereas 
the VBT panel type (2) gives us normal
aeddda06 Ville Syrjälä 2016-07-12  1078  * vswing instead. Low vswing 
results in some display flickers, so
aeddda06 Ville Syrjälä 2016-07-12  1079  * let's simply ignore the 
OpRegion panel type on SKL for now.
aeddda06 Ville Syrjälä 2016-07-12  1080  */
aeddda06 Ville Syrjälä 2016-07-12 @1081 if (IS_SKYLAKE(dev)) {
aeddda06 Ville Syrjälä 2016-07-12  1082 DRM_DEBUG_KMS("Ignoring 
OpRegion panel type (%d)\n", ret - 1);
aeddda06 Ville Syrjälä 2016-07-12  1083 return 

Re: [Intel-gfx] [PATCH] drm/i915: Fix legacy gamma lut updates in Linux 4.7-rc6

[Mario Kleiner]

Ok, so legacy gamma table updates are completely broken for Intel on 
Linux-4.7-rc7, the final release candidate.


The good news is that applying Lionel's patch

"drm/i915: add missing condition for committing planes on crtc"

from

https://patchwork.freedesktop.org/patch/89111/

fixes it nicely. The patch currently applies cleanly to drm-fixes and 
drm-next and is


Reviewed-and-tested-by: Mario Kleiner 


When we are at it, could somebody please look at that updated series of 
my Displayport color depth fixes ("EDID/DP fixes for proper bpc 
detection of displays.") i sent out a week ago?


Especially pulling patch 2/5 "[PATCH 2/5] drm/i915/dp: Revert 
"drm/i915/dp: fall back to 18 bpp when sink capability is unknown" would 
be important, as that bug introduced a regression for Intel + DP + 
legacy DP converters into stable kernels which is very serious for users 
of scientific/medical display equipment, especially as the failures can 
easily go unnoticed during normal equipment tests, but would introduce 
the equivalent of "silent data corruption" into their measured 
scientific data, which is not a great experience given that collecting 
such data can easily take half a year of work time and ten-thousands of 
euros of wasted research funding.


Patches 3 and 4 contain changes Daniel asked me to do, patch 5 would be 
good to safe-guard against similar issues in the future.


thanks,
-mario

On 07/12/2016 12:50 PM, Lionel Landwerlin wrote:

Hi Mario,

There was a couple of patch to fix this issue :

https://patchwork.freedesktop.org/series/5467/
https://patchwork.freedesktop.org/series/5466/

I tested this late last week on drm-intel-nightly, it seems a series of
revert fixed most of the issues.

Cheers,

-
Lionel

On 12/07/16 11:33, Mario Kleiner wrote:

Updating legacy gamma tables, e.g., via RandR doesn't work at all
as of Linux 4.7-rc6.

Reason seems to be that the required call to
drm_atomic_helper_commit_planes_on_crtc is skipped in
intel_atomic_commit after userspace set new gamma tables,
because neither crtc->state->planes_changed nor
update_pipe (= pipe_config->update_pipe) are true.

Removing the check for planes_changed || update_pipe fixes
gamma table updates.

The code for Linux 4.8 drm-next has changed a lot in that area
wrt. 4.7, but the new code for 4.8 also removed those checks
and calls drm_atomic_helper_commit_planes_on_crtc unconditionally,
and legacy gamma lut updates work on drm-next, so this seems to be
the right solution.

Tested also shutdown/reboot, suspend/resume, (un-)plugging displays,
mode switches for resolution/refresh rate, display rotation, and
page-flipping/pageflip timing on Intel HD Ironlake to confirm the
fix apparently doesn't break anything under X11.

Signed-off-by: Mario Kleiner 
Cc: Maarten Lankhorst 
Cc: Lionel Landwerlin 
Cc: Daniel Vetter 
---
  drivers/gpu/drm/i915/intel_display.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 04452cf..eb8fb36 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13685,7 +13685,6 @@ static int intel_atomic_commit(struct
drm_device *dev,
  bool modeset = needs_modeset(crtc->state);
  struct intel_crtc_state *pipe_config =
  to_intel_crtc_state(crtc->state);
-bool update_pipe = !modeset && pipe_config->update_pipe;
  if (modeset && crtc->state->active) {
  update_scanline_offset(to_intel_crtc(crtc));
@@ -13699,8 +13698,7 @@ static int intel_atomic_commit(struct
drm_device *dev,
  drm_atomic_get_existing_plane_state(state, crtc->primary))
  intel_fbc_enable(intel_crtc);
-if (crtc->state->active &&
-(crtc->state->planes_changed || update_pipe))
+if (crtc->state->active)
  drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
  if (pipe_config->base.active && needs_vblank_wait(pipe_config))




___
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: Check live status before reading edid

[David Weinehall]

On Wed, Jul 13, 2016 at 01:17:40PM +0100, Chris Wilson wrote:
> On Wed, Jul 13, 2016 at 01:09:28PM +0100, Chris Wilson wrote:
> > On Wed, Jul 13, 2016 at 01:59:52PM +0200, Daniel Vetter wrote:
> > > I think the proper solution should be to make all the probing and fbdev
> > > restoring async. As soon as we have working async atomic commit for
> > > modeset we should be able to do all that.
> > 
> > We're not just blocked on the mode change here (indeed, we shouldn't be
> > changing modes at resume at all, right?) but we appear to be doing a
> > full detection cycle whereas the intent was just to tell userspace
> > everything had changed, and for it to go figure out what to do about it.
> > 
> > Also note that we can simply move this all out of the blocking resume
> > path and still run this in parallel to userspace resuming (and of course
> > serialised with whatever userspace wants to do).
> 
> And to remind myself of conversations going on elsewhere, the more async
> we make resume the more inaccurate the current method of measuing resume
> time becomes (which more or less just looks at the initcall graph). We
> need a metric that measures the time from resume to the time of first
> pixel (first flip to a lit display preferably). I've shown how we can
> get our "resume time" down to about 10ms - all because the metric is
> subject to abuse.

Good news on this front -- it seems that the SuspendResume tool can be
adapted to measure our resume-time even if we "cheat", and the author
offered to help with this.  So we "just" need to decide what actually
constitutes being done with resume.


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

Re: [Intel-gfx] [PATCH i-g-t] tests: Fix compilation when some gcc configurations

[Chris Wilson]

On Thu, Jul 14, 2016 at 06:44:59PM +0100, Chris Wilson wrote:
> On Thu, Jul 14, 2016 at 06:31:37PM +0100, Damien Lespiau wrote:
> > Depending how the gcc was compiled it may be necessary to enable SSE4
> > instructions explicitly. Otherwise:
> > 
> >   CC   gem_gtt_speed.o
> > In file included from gem_gtt_speed.c:54:0:
> > /usr/lib/gcc/x86_64-linux-gnu/4.8/include/smmintrin.h:31:3: error: #error 
> > "SSE4.1 instruction set not enabled"
> >  # error "SSE4.1 instruction set not enabled"
> 
> So the challenge is getting the function attribute applied to the
> include.
> 
> Ah, can you try
> #pragma GCC target ("sse4.1")
> in those blocks instead?

Oh, and we probably need an #if GCC > 4.y to be fully backwards
compatible.
-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] tests: Fix compilation when some gcc configurations

[Chris Wilson]

On Thu, Jul 14, 2016 at 06:31:37PM +0100, Damien Lespiau wrote:
> Depending how the gcc was compiled it may be necessary to enable SSE4
> instructions explicitly. Otherwise:
> 
>   CC   gem_gtt_speed.o
> In file included from gem_gtt_speed.c:54:0:
> /usr/lib/gcc/x86_64-linux-gnu/4.8/include/smmintrin.h:31:3: error: #error 
> "SSE4.1 instruction set not enabled"
>  # error "SSE4.1 instruction set not enabled"

So the challenge is getting the function attribute applied to the
include.

Ah, can you try
#pragma GCC target ("sse4.1")
in those blocks instead?

> @@ -76,6 +76,7 @@ gem_ctx_basic_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
>  gem_ctx_basic_LDADD = $(LDADD) -lpthread
>  gem_ctx_thrash_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
>  gem_ctx_thrash_LDADD = $(LDADD) -lpthread
> +gem_exec_flush_CFLAGS = $(AM_CFLAGS) -msse4

Note, it should be -msse4.1
-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

[Intel-gfx] [PATCH i-g-t] tests: Fix compilation when some gcc configurations

[Damien Lespiau]

Depending how the gcc was compiled it may be necessary to enable SSE4
instructions explicitly. Otherwise:

  CC   gem_gtt_speed.o
In file included from gem_gtt_speed.c:54:0:
/usr/lib/gcc/x86_64-linux-gnu/4.8/include/smmintrin.h:31:3: error: #error 
"SSE4.1 instruction set not enabled"
 # error "SSE4.1 instruction set not enabled"
   ^
gem_gtt_speed.c: In function ‘streaming_load’:
gem_gtt_speed.c:59:2: error: unknown type name ‘__m128i’
  __m128i tmp, *s = src;
  ^
gem_gtt_speed.c:65:3: error: implicit declaration of function 
‘_mm_stream_load_si128’ [-Werror=implicit-function-declaration]
   tmp += _mm_stream_load_si128(s++);
   ^
gem_gtt_speed.c:65:3: warning: nested extern declaration of 
‘_mm_stream_load_si128’ [-Wnested-externs]
gem_gtt_speed.c:70:2: error: unknown type name ‘__m128i’
  *(volatile __m128i *)src = tmp;
  ^

CC: Chris Wilson 
Signed-off-by: Damien Lespiau 
---
 tests/Makefile.am | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 102b8a6..8c6b0a3 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -76,6 +76,7 @@ gem_ctx_basic_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
 gem_ctx_basic_LDADD = $(LDADD) -lpthread
 gem_ctx_thrash_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
 gem_ctx_thrash_LDADD = $(LDADD) -lpthread
+gem_exec_flush_CFLAGS = $(AM_CFLAGS) -msse4
 gem_exec_parallel_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
 gem_exec_parallel_LDADD = $(LDADD) -lpthread
 gem_fence_thrash_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
@@ -84,6 +85,7 @@ gem_fence_upload_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
 gem_fence_upload_LDADD = $(LDADD) -lpthread
 gem_flink_race_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
 gem_flink_race_LDADD = $(LDADD) -lpthread
+gem_gtt_speed_CFLAGS = $(AM_CFLAGS) -msse4
 gem_mmap_gtt_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
 gem_mmap_gtt_LDADD = $(LDADD) -lpthread
 gem_mmap_wc_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
-- 
2.5.0

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

Re: [Intel-gfx] [PATCH] Bump libdrm-intel dependency library to a newer version that support softpin.

[Chris Wilson]

On Thu, Jul 14, 2016 at 05:29:55PM +0100, Emil Velikov wrote:
> Hi Marius,
> 
> Just double-checking - this is an i-g-t patch, isn't it ?
> 
> On 14 July 2016 at 11:39, Marius Vlad  wrote:
> > Required by commit 2603b98ca (aubdump: Support softpin bos).
> >
> > Signed-off-by: Marius Vlad 
> > CC: Kristian Høgsberg Kristensen 
> > ---
> >  configure.ac | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index f05bcb0..ade9756 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -119,7 +119,7 @@ if test "x$GCC" = "xyes"; then
> >  fi
> >  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
> >
> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm])
> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.68 libdrm])
> Yes please. As you do that one can nuke most/all the "define LOCAL_"
> and "struct local_" (in lib/ioctl_wrappers.h)
> and replace with with the official symbols. A very nice plus imho ;-)

Please don't. It makes running on older installations even more
cumbersome.
-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] Bump libdrm-intel dependency library to a newer version that support softpin.

[Emil Velikov]

Hi Marius,

Just double-checking - this is an i-g-t patch, isn't it ?

On 14 July 2016 at 11:39, Marius Vlad  wrote:
> Required by commit 2603b98ca (aubdump: Support softpin bos).
>
> Signed-off-by: Marius Vlad 
> CC: Kristian Høgsberg Kristensen 
> ---
>  configure.ac | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index f05bcb0..ade9756 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -119,7 +119,7 @@ if test "x$GCC" = "xyes"; then
>  fi
>  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
>
> -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm])
> +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.68 libdrm])
Yes please. As you do that one can nuke most/all the "define LOCAL_"
and "struct local_" (in lib/ioctl_wrappers.h)
and replace with with the official symbols. A very nice plus imho ;-)

Side note: this will conflict with Robert Foss's work to make
libdrm_intel an optional dependency. Have you/others had the chance to
look at the series ? What can we do to get that moving/accepted ?

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

[Intel-gfx] [drm-intel:drm-intel-nightly 2/10] drivers/gpu/drm/i915/intel_opregion.c:1081:17: error: 'dev' undeclared

[kbuild test robot]

tree:   git://anongit.freedesktop.org/drm-intel drm-intel-nightly
head:   2d854c67e3af36b190e8499a3bfad7cdccde0f67
commit: 01734c300fbff01e321d4ff1b3c91e24e0a3b90d [2/10] Merge remote-tracking 
branch 'origin/drm-intel-next-fixes' into drm-intel-nightly
config: i386-randconfig-i1-201628 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
git checkout 01734c300fbff01e321d4ff1b3c91e24e0a3b90d
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from drivers/gpu/drm/i915/intel_opregion.c:34:0:
   drivers/gpu/drm/i915/intel_opregion.c: In function 
'intel_opregion_get_panel_type':
>> drivers/gpu/drm/i915/intel_opregion.c:1081:17: error: 'dev' undeclared 
>> (first use in this function)
 if (IS_SKYLAKE(dev)) {
^
   drivers/gpu/drm/i915/i915_drv.h:2628:43: note: in definition of macro 
'__I915__'
 if (__builtin_types_compatible_p(typeof(*p), struct drm_i915_private)) \
  ^
   drivers/gpu/drm/i915/i915_drv.h:2695:26: note: in expansion of macro 
'INTEL_INFO'
#define IS_SKYLAKE(dev) (INTEL_INFO(dev)->is_skylake)
 ^~
   drivers/gpu/drm/i915/intel_opregion.c:1081:6: note: in expansion of macro 
'IS_SKYLAKE'
 if (IS_SKYLAKE(dev)) {
 ^~
   drivers/gpu/drm/i915/intel_opregion.c:1081:17: note: each undeclared 
identifier is reported only once for each function it appears in
 if (IS_SKYLAKE(dev)) {
^
   drivers/gpu/drm/i915/i915_drv.h:2628:43: note: in definition of macro 
'__I915__'
 if (__builtin_types_compatible_p(typeof(*p), struct drm_i915_private)) \
  ^
   drivers/gpu/drm/i915/i915_drv.h:2695:26: note: in expansion of macro 
'INTEL_INFO'
#define IS_SKYLAKE(dev) (INTEL_INFO(dev)->is_skylake)
 ^~
   drivers/gpu/drm/i915/intel_opregion.c:1081:6: note: in expansion of macro 
'IS_SKYLAKE'
 if (IS_SKYLAKE(dev)) {
 ^~

vim +/dev +1081 drivers/gpu/drm/i915/intel_opregion.c

a0562819 Ville Syrjälä 2016-04-11  1065 DRM_DEBUG_KMS("Invalid 
OpRegion panel type 0x%x\n", ret);
a0562819 Ville Syrjälä 2016-04-11  1066 return -EINVAL;
a0562819 Ville Syrjälä 2016-04-11  1067 }
a0562819 Ville Syrjälä 2016-04-11  1068  
a0562819 Ville Syrjälä 2016-04-11  1069 /* fall back to VBT panel type? 
*/
a0562819 Ville Syrjälä 2016-04-11  1070 if (ret == 0x0) {
a0562819 Ville Syrjälä 2016-04-11  1071 DRM_DEBUG_KMS("No panel 
type in OpRegion\n");
a0562819 Ville Syrjälä 2016-04-11  1072 return -ENODEV;
a0562819 Ville Syrjälä 2016-04-11  1073 }
a0562819 Ville Syrjälä 2016-04-11  1074  
aeddda06 Ville Syrjälä 2016-07-12  1075 /*
aeddda06 Ville Syrjälä 2016-07-12  1076  * FIXME On Dell XPS 13 9350 
the OpRegion panel type (0) gives us
aeddda06 Ville Syrjälä 2016-07-12  1077  * low vswing for eDP, whereas 
the VBT panel type (2) gives us normal
aeddda06 Ville Syrjälä 2016-07-12  1078  * vswing instead. Low vswing 
results in some display flickers, so
aeddda06 Ville Syrjälä 2016-07-12  1079  * let's simply ignore the 
OpRegion panel type on SKL for now.
aeddda06 Ville Syrjälä 2016-07-12  1080  */
aeddda06 Ville Syrjälä 2016-07-12 @1081 if (IS_SKYLAKE(dev)) {
aeddda06 Ville Syrjälä 2016-07-12  1082 DRM_DEBUG_KMS("Ignoring 
OpRegion panel type (%d)\n", ret - 1);
aeddda06 Ville Syrjälä 2016-07-12  1083 return -ENODEV;
aeddda06 Ville Syrjälä 2016-07-12  1084 }
aeddda06 Ville Syrjälä 2016-07-12  1085  
a0562819 Ville Syrjälä 2016-04-11  1086 return ret - 1;
a0562819 Ville Syrjälä 2016-04-11  1087  }

:: The code at line 1081 was first introduced by commit
:: aeddda06c1a704bb97c8a7bfe7a472120193bd56 drm/i915: Ignore panel type 
from OpRegion on SKL

:: TO: Ville Syrjälä 
:: CC: Daniel Vetter 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH driver/intel] uxa: Don't bother with xf86GARTCloseScreen

[Adam Jackson]

We only ever use xserver's AGP support from the i810 driver.

Signed-off-by: Adam Jackson 
---
 src/uxa/intel_driver.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/uxa/intel_driver.c b/src/uxa/intel_driver.c
index 73d7f4f..62abdd2 100644
--- a/src/uxa/intel_driver.c
+++ b/src/uxa/intel_driver.c
@@ -1172,8 +1172,6 @@ static Bool I830CloseScreen(CLOSE_SCREEN_ARGS_DECL)
 
intel_sync_close(screen);
 
-   xf86GARTCloseScreen(scrn->scrnIndex);
-
scrn->vtSema = FALSE;
return TRUE;
 }
-- 
2.7.4

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

Re: [Intel-gfx] [PATCH v3] drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl)

[Chris Wilson]

On Thu, Jul 14, 2016 at 04:36:37PM +0200, Daniel Vetter wrote:
> On Thu, Jul 14, 2016 at 02:39:54PM +0100, Chris Wilson wrote:
> > On Thu, Jul 14, 2016 at 02:23:04PM +0100, Chris Wilson wrote:
> > > The biggest reason I had against going the sw_sync only route was that
> > > vgem should provide unprivileged fences and that through the bookkeeping
> > > in vgem we can keep them safe, ensure that we don't leak random buffers
> > > or fences. (And I need a source of foriegn dma-buf with implicit fence
> > > tracking with which I can try and break the driver.)
> > 
> > And for testing passing around content + fences is more useful than
> > passing fences alone.
> 
> Yup, agreed. But having fences free-standing isn't a real issue since
> their refcounted and the userspace parts (sync_file) will get cleaned up
> on process exit latest. Ḯ'm not advocating for any behaviour change at
> all, just for hiding these things in debugfs.

It's just a choice of api. We could equally hide it behind a separate
config flag.

First question, are we happy that there is a legitimate usecase for fences
on vgem?

If so, what enforced timeout on the fence should we use?

(I think that this ioctl api is correct, I don't forsee sw_sync being
viable for unprivileged use.)

Then we can restrict this patch to add the safe interface, enable a bunch
more tests and get on with discussing how to break the kernel "safely"!
-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 1/2] drm/i915/fbdev: Drain the suspend worker on retiring

[Daniel Vetter]

On Thu, Jul 14, 2016 at 01:47:49PM +0300, Mika Kuoppala wrote:
> Chris Wilson  writes:
> 
> > Since the suspend_work can arm itself if the console_lock() is currently
> > held elsewhere, simply calling flush_work() doesn't guarantee that the
> > work is idle upon return. To do so requires using cancel_work_sync().
> >
> > Signed-off-by: Chris Wilson 
> > Cc: Daniel Vetter 
> > Cc: Mika Kuoppala 
> 
> Reviewed-by: Mika Kuoppala 

Cc: sta...@vger.kernel.org I presume? Checking for this should be part of
the review ...
-Daniel

> 
> > ---
> >  drivers/gpu/drm/i915/intel_fbdev.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c 
> > b/drivers/gpu/drm/i915/intel_fbdev.c
> > index 86b00c6db1a6..ef17d88a1bc7 100644
> > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > @@ -768,7 +768,7 @@ void intel_fbdev_fini(struct drm_device *dev)
> > if (!ifbdev)
> > return;
> >  
> > -   flush_work(_priv->fbdev_suspend_work);
> > +   cancel_work_sync(_priv->fbdev_suspend_work);
> > if (!current_is_async())
> > intel_fbdev_sync(ifbdev);
> >  
> > -- 
> > 2.8.1

-- 
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/2] drm/i915/fbdev: Drain the suspend worker on retiring

[Chris Wilson]

On Thu, Jul 14, 2016 at 04:45:52PM +0200, Daniel Vetter wrote:
> On Thu, Jul 14, 2016 at 01:47:49PM +0300, Mika Kuoppala wrote:
> > Chris Wilson  writes:
> > 
> > > Since the suspend_work can arm itself if the console_lock() is currently
> > > held elsewhere, simply calling flush_work() doesn't guarantee that the
> > > work is idle upon return. To do so requires using cancel_work_sync().
> > >
> > > Signed-off-by: Chris Wilson 
> > > Cc: Daniel Vetter 
> > > Cc: Mika Kuoppala 
> > 
> > Reviewed-by: Mika Kuoppala 
> 
> Cc: sta...@vger.kernel.org I presume? Checking for this should be part of
> the review ...

No known bug - unlikely since ~nobody unloads the module.
-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 2/2] drm/i915/fbdev: Check for the framebuffer before use

[Daniel Vetter]

On Wed, Jul 13, 2016 at 06:34:45PM +0100, Chris Wilson wrote:
> If the fbdev probing fails, and in our error path we fail to clear the
> dev_priv->fbdev, then we can try and use a dangling fbdev pointer, and
> in particular a NULL fb. This could also happen in pathological cases
> where we try to operate on the fbdev prior to it being probed.
> 
> Reported-by: Maarten Lankhorst 
> Signed-off-by: Chris Wilson 
> Cc: Maarten Lankhorst 
> Cc: Daniel Vetter 
> Cc: Mika Kuoppala 

Thierry Redding has a patch series in flight for generic delayed fbdev
setup, whether it's delayed to due async init or because there's nothing
yet connected (which some of the embedded drivers hack together). With
those patches most of these checks should become unecessary again.

Adding Thierry so he's aware that there's more to do when rebasing, and so
he knows where he can find reviewers ;-)

Reviewed-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/i915/intel_fbdev.c | 25 +++--
>  1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c 
> b/drivers/gpu/drm/i915/intel_fbdev.c
> index ef17d88a1bc7..23129dcfba9d 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -782,7 +782,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int 
> state, bool synchronous
>   struct intel_fbdev *ifbdev = dev_priv->fbdev;
>   struct fb_info *info;
>  
> - if (!ifbdev)
> + if (!ifbdev || !ifbdev->fb)
>   return;
>  
>   info = ifbdev->helper.fbdev;
> @@ -827,31 +827,28 @@ void intel_fbdev_set_suspend(struct drm_device *dev, 
> int state, bool synchronous
>  
>  void intel_fbdev_output_poll_changed(struct drm_device *dev)
>  {
> - struct drm_i915_private *dev_priv = to_i915(dev);
> - if (dev_priv->fbdev)
> - drm_fb_helper_hotplug_event(_priv->fbdev->helper);
> + struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
> +
> + if (ifbdev && ifbdev->fb)
> + drm_fb_helper_hotplug_event(>helper);
>  }
>  
>  void intel_fbdev_restore_mode(struct drm_device *dev)
>  {
> - int ret;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_fbdev *ifbdev = dev_priv->fbdev;
> - struct drm_fb_helper *fb_helper;
> + struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
>  
>   if (!ifbdev)
>   return;
>  
>   intel_fbdev_sync(ifbdev);
> + if (!ifbdev->fb)
> + return;
>  
> - fb_helper = >helper;
> -
> - ret = drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper);
> - if (ret) {
> + if (drm_fb_helper_restore_fbdev_mode_unlocked(>helper)) {
>   DRM_DEBUG("failed to restore crtc mode\n");
>   } else {
> - mutex_lock(_helper->dev->struct_mutex);
> + mutex_lock(>struct_mutex);
>   intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT);
> - mutex_unlock(_helper->dev->struct_mutex);
> + mutex_unlock(>struct_mutex);
>   }
>  }
> -- 
> 2.8.1
> 

-- 
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/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL

[Daniel Vetter]

On Wed, Jul 13, 2016 at 04:32:03PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Bspec tells us to keep bashing the PCU for up to 3ms when trying to
> inform it about an upcoming change in the cdclk frequency. Currently
> we only keep at it for 15*10usec (+ whatever delays gets added by
> the sandybridge_pcode_read() itself). Let's change the limit to 3ms.
> 
> I decided to keep 10 usec delay per iteration for now, even though
> the spec doesn't really tell us to do that.
> 
> Cc: David Weinehall 
> Signed-off-by: Ville Syrjälä 

Needs

Cc: sta...@vger.kernel.org

and probably also a Fixes: line?
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 10 +-
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index be3b2cab2640..90f26f0e2571 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5691,15 +5691,7 @@ static bool skl_cdclk_pcu_ready(struct 
> drm_i915_private *dev_priv)
>  
>  static bool skl_cdclk_wait_for_pcu_ready(struct drm_i915_private *dev_priv)
>  {
> - unsigned int i;
> -
> - for (i = 0; i < 15; i++) {
> - if (skl_cdclk_pcu_ready(dev_priv))
> - return true;
> - udelay(10);
> - }
> -
> - return false;
> + return _wait_for(skl_cdclk_pcu_ready(dev_priv), 3000, 10) == 0;
>  }
>  
>  static void skl_set_cdclk(struct drm_i915_private *dev_priv, int cdclk, int 
> vco)
> -- 
> 2.7.4
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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 3/6] drm/i915/huc: Add HuC fw loading support

[Peter Antoine]

On Thu, 14 Jul 2016, Daniel Vetter wrote:


On Wed, Jul 13, 2016 at 03:52:39PM +0100, Peter Antoine wrote:

On Wed, 13 Jul 2016, Daniel Vetter wrote:


On Thu, Jun 23, 2016 at 02:52:41PM +0100, Peter Antoine wrote:

On Thu, 23 Jun 2016, Dave Gordon wrote:

On 22/06/16 09:31, Daniel Vetter wrote:
No, the *correct* fix is to unify all the firmware loaders we have.
There should just be ONE piece of code that can be used to fetch and
load ANy firmware into ANY auxiliary microcontroller. NOT one per
microcontroller, all different -- that way lies madness.

We already had a unified loader for the HuC and GuC a year ago, but IIRC
the party line then was "just make it (GuC) specific, then copypaste it
for the second uC, and when we've got three versions we'll have learnt
how we really want a unified loader to behave."

Well. here's the copypaste, and we already have a different loader for
the DMC/CSR, so it must be time for (re-)unification.

.Dave.



Just to add, if you uc_fw_fetch() has an error code you will still have to
remember the state of the fetch or at each reset/resume/etc... or you will
have to try the firmware load again and that can take a long time. So the
state will have to be re-instated.

Seeing this code was written with the given goals and were written in the
same vane as code that was deemed acceptable, it seems weird at this late
stage to change the design goals.

Note: this is the third time that these patches have been posted and were
only rejected (as far as I know) due to no open-source user. Which there is
now, and is why I have reposted these patches.


I never liked the guc firmware code, but figure for one copy it's not
worth fighting over. Adding more copies (or perpetuating the design by
making it generic) isn't what I'm looking for. Firmware loading shouldn't
be that complicated, really.

The unified firmware loader is called request_firmware. If that's not good
enough, pls fix the core function, not paper code over in i915. In that
regard DMC/CSR is unified, everything else isn't yet.

Iirc the big issue is delayed firmware loading for built-in i915 and fw
only available later on. This is an open issue in request_firmware() since
years, and there's various patches floating around. If the problem is that
Greg KH doesn't consider those patches, I can help with that. But not
pushing the core fix forward isn't acceptable imo. Once that fix is landed
we can treat request_firmware as reliable (it might take a while, hence
must be run in an async work like DMC loading), with no need to ever retry
anything. If fw loading fails we can just mark the entire render part of
the gpu as dead by injecting the equivalent of a non-recoverable hang
(async setup) or failing engine init with -EIO (if this is still
synchronous, which I don't expect really).

If there's another reason for this complexity, please explain since I'd
like to understand why we need this.
-Daniel



I was not involved at the start and I am porting code that others have
written, but as far as I understand this, you requested that the code be
duplicated. See Dave's comment above as he was involved.

The code uses request_firmware() to handle the load of the firmware into
memory and the rest of this code manages the loading of that memory into the
HuC's SRAM. This needs extra setup that should not really go into the
generic firmware loader (one loader for uIA's make sense as this will
futureproof the code). Also the SRAM is write-once memory and needs to be
handled correctly. Also, the GuC needs to verify the HuC some this becomes a
little be more fun.

Also, again you are ignoring the point that not having firmware is not
fatal. We remember the state of the load as this is required to save time
when we come out of reset to not waste time trying to reload the firmware,
if it has failed already. They are other mechinums to do this, but they will
always need some form of history.

Also, if request_firmware() is broken why has this not been fixed? If it has
been broken for "years" why and how do you expect to to be fixed now? If the
"not pushing the core fix is not acceptable" why has that not been done?


Apparently because I'm a too nice maintainer and allowed half-solutions to
get landed, under the expecations that people would indeed follow up and
fix things.

And yes, you care, you fix it, is how this works, whether you like it or
not.
-Daniel


Daniel,

I won't have to time to do this. So I'll leave the HuC/GuC rewrite for 
you to find someone else to take them on as this is likely to take more 
time than I will be employed by Intel.


Peter.


--
   Peter Antoine (Android Graphics Driver Software Engineer)
   -
   Intel Corporation (UK) Limited
   Registered No. 1134945 (England)
   Registered Office: Pipers Way, Swindon SN3 1RJ
   VAT No: 860 2173 47
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org

Re: [Intel-gfx] [PATCH v3] drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl)

[Daniel Vetter]

On Thu, Jul 14, 2016 at 02:39:54PM +0100, Chris Wilson wrote:
> On Thu, Jul 14, 2016 at 02:23:04PM +0100, Chris Wilson wrote:
> > The biggest reason I had against going the sw_sync only route was that
> > vgem should provide unprivileged fences and that through the bookkeeping
> > in vgem we can keep them safe, ensure that we don't leak random buffers
> > or fences. (And I need a source of foriegn dma-buf with implicit fence
> > tracking with which I can try and break the driver.)
> 
> And for testing passing around content + fences is more useful than
> passing fences alone.

Yup, agreed. But having fences free-standing isn't a real issue since
their refcounted and the userspace parts (sync_file) will get cleaned up
on process exit latest. Ḯ'm not advocating for any behaviour change at
all, just for hiding these things in debugfs.

Or maybe we could add a special (tainting) module option to vgem.ko which
enables this interface? That would be even less work, can easily be
integrated into igt (just set that knob at runtime, done), and with a
stern enough warning in dmesg + tainting the point should be clear. Of
course that switch would be off by default. Thoughts?
-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 3/6] drm/i915/huc: Add HuC fw loading support

[Dave Gordon]

On 14/07/16 15:16, Daniel Vetter wrote:

On Wed, Jul 13, 2016 at 03:52:39PM +0100, Peter Antoine wrote:

On Wed, 13 Jul 2016, Daniel Vetter wrote:


On Thu, Jun 23, 2016 at 02:52:41PM +0100, Peter Antoine wrote:

On Thu, 23 Jun 2016, Dave Gordon wrote:

On 22/06/16 09:31, Daniel Vetter wrote:
No, the *correct* fix is to unify all the firmware loaders we have.
There should just be ONE piece of code that can be used to fetch and
load ANy firmware into ANY auxiliary microcontroller. NOT one per
microcontroller, all different -- that way lies madness.

We already had a unified loader for the HuC and GuC a year ago, but IIRC
the party line then was "just make it (GuC) specific, then copypaste it
for the second uC, and when we've got three versions we'll have learnt
how we really want a unified loader to behave."

Well. here's the copypaste, and we already have a different loader for
the DMC/CSR, so it must be time for (re-)unification.

.Dave.


Just to add, if you uc_fw_fetch() has an error code you will still have to
remember the state of the fetch or at each reset/resume/etc... or you will
have to try the firmware load again and that can take a long time. So the
state will have to be re-instated.

Seeing this code was written with the given goals and were written in the
same vane as code that was deemed acceptable, it seems weird at this late
stage to change the design goals.

Note: this is the third time that these patches have been posted and were
only rejected (as far as I know) due to no open-source user. Which there is
now, and is why I have reposted these patches.


I never liked the guc firmware code, but figure for one copy it's not
worth fighting over. Adding more copies (or perpetuating the design by
making it generic) isn't what I'm looking for. Firmware loading shouldn't
be that complicated, really.

The unified firmware loader is called request_firmware. If that's not good
enough, pls fix the core function, not paper code over in i915. In that
regard DMC/CSR is unified, everything else isn't yet.

Iirc the big issue is delayed firmware loading for built-in i915 and fw
only available later on. This is an open issue in request_firmware() since
years, and there's various patches floating around. If the problem is that
Greg KH doesn't consider those patches, I can help with that. But not
pushing the core fix forward isn't acceptable imo. Once that fix is landed
we can treat request_firmware as reliable (it might take a while, hence
must be run in an async work like DMC loading), with no need to ever retry
anything. If fw loading fails we can just mark the entire render part of
the gpu as dead by injecting the equivalent of a non-recoverable hang
(async setup) or failing engine init with -EIO (if this is still
synchronous, which I don't expect really).

If there's another reason for this complexity, please explain since I'd
like to understand why we need this.
-Daniel


I was not involved at the start and I am porting code that others have
written, but as far as I understand this, you requested that the code be
duplicated. See Dave's comment above as he was involved.

The code uses request_firmware() to handle the load of the firmware into
memory and the rest of this code manages the loading of that memory into the
HuC's SRAM. This needs extra setup that should not really go into the
generic firmware loader (one loader for uIA's make sense as this will
futureproof the code). Also the SRAM is write-once memory and needs to be
handled correctly. Also, the GuC needs to verify the HuC some this becomes a
little be more fun.

Also, again you are ignoring the point that not having firmware is not
fatal. We remember the state of the load as this is required to save time
when we come out of reset to not waste time trying to reload the firmware,
if it has failed already. They are other mechinums to do this, but they will
always need some form of history.

Also, if request_firmware() is broken why has this not been fixed? If it has
been broken for "years" why and how do you expect to to be fixed now? If the
"not pushing the core fix is not acceptable" why has that not been done?


Apparently because I'm a too nice maintainer and allowed half-solutions to
get landed, under the expecations that people would indeed follow up and
fix things.

And yes, you care, you fix it, is how this works, whether you like it or
not.
-Daniel


AFAIK request_firmware() is *not* broken. It does what it says i.e. 
fetches a blob from the internal bucket or from the filesystem; and 
fails if it's not found. So it doesn't need fixing.


What's broken (from the Android POV) is i915 needing to do too much too 
early i.e. before all filesystems are mounted. The answer to *that* is 
to defer *all* engine initialisation until (much) later; but that's 
nothing at all to do with Peter's patches here.


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

Re: [Intel-gfx] [PATCH] drm/i915: Acquire intel_runtime_pm for HD-Audio registers

[Chris Wilson]

On Wed, Jul 13, 2016 at 05:01:02PM +0300, Marius Vlad wrote:
> Did try when you submitted the patch...but can't seem to replicate with
> latest nightly on other SKLs, and currently do not have access on
> the machine that caused it.

So fwiw, Hans de Geode confirmed that only reverting 03b135cebc47 (which
enables the display powerwell as well) was enough.

Libin, do you want to handle the revert or explain the mistake in

commit 03b135cebc47d75ea2dc346770374ab741966955
Author: Libin Yang 
Date:   Wed Jun 3 09:30:15 2015 +0800

ALSA: hda - remove controller dependency on i915 power well for SKL

-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 v3] drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl)

[Daniel Vetter]

On Thu, Jul 14, 2016 at 02:23:04PM +0100, Chris Wilson wrote:
> On Thu, Jul 14, 2016 at 02:40:59PM +0200, Daniel Vetter wrote:
> > On Thu, Jul 14, 2016 at 11:11:02AM +0100, Chris Wilson wrote:
> > > On Thu, Jul 14, 2016 at 10:59:04AM +0100, Chris Wilson wrote:
> > > > On Thu, Jul 14, 2016 at 10:12:17AM +0200, Daniel Vetter wrote:
> > > > > On Thu, Jul 14, 2016 at 08:04:19AM +0100, Chris Wilson wrote:
> > > > > > vGEM buffers are useful for passing data between software clients 
> > > > > > and
> > > > > > hardware renders. By allowing the user to create and attach fences 
> > > > > > to
> > > > > > the exported vGEM buffers (on the dma-buf), the user can implement a
> > > > > > deferred renderer and queue hardware operations like flipping and 
> > > > > > then
> > > > > > signal the buffer readiness (i.e. this allows the user to schedule
> > > > > > operations out-of-order, but have them complete in-order).
> > > > > > 
> > > > > > This also makes it much easier to write tightly controlled 
> > > > > > testcases for
> > > > > > dma-buf fencing and signaling between hardware drivers.
> > > > > > 
> > > > > > v2: Don't pretend the fences exist in an ordered timeline, but 
> > > > > > allocate
> > > > > > a separate fence-context for each fence so that the fences are
> > > > > > unordered.
> > > > > > v3: Make the debug output more interesting, and so the signaled 
> > > > > > status.
> > > > > > 
> > > > > > Testcase: igt/vgem_basic/dmabuf-fence
> > > > > > Signed-off-by: Chris Wilson 
> > > > > > Cc: Sean Paul 
> > > > > > Cc: Zach Reizner 
> > > > > > Cc: Gustavo Padovan 
> > > > > > Cc: Daniel Vetter 
> > > > > > Acked-by: Zach Reizner 
> > > > > 
> > > > > One thing I completely forgotten: This allows userspace to hang kernel
> > > > > drivers. i915 (and other gpu drivers) can recover using hangcheck, but
> > > > > dumber drivers (v4l, if that ever happens) probably never except such 
> > > > > a
> > > > > case. We've had a similar discusion with the userspace fences exposed 
> > > > > in
> > > > > sw_fence, and decided to move all those ioctl into debugfs. I think we
> > > > > should do the same for this vgem-based debugging of implicit sync. 
> > > > > Sorry
> > > > > for realizing this this late.
> > > > 
> > > > One of the very tests I make is to ensure that we recover from such a
> > > > hang. I don't see the difference between this any of the other ways
> > > > userspace can shoot itself (and others) in the foot.
> > > 
> > > So one solution would be to make vgem fences automatically timeout (with
> > > a flag for root to override for the sake of testing hang detection).
> > 
> > The problem is other drivers. E.g. right now atomic helpers assume that
> > fences will signal, and can't recover if they don't. This is why drivers
> > where things might fail must have some recovery (hangcheck, timeout) to
> > make sure dma_fences always signal.
> 
> Urm, all the atomic helpers should work with fails. The waits on dma-buf
> should be before any hardware is modified and so cancellation is trivial.
> Anyone using a foriegn fence (or even native) must cope that it may not
> meet some deadline.
> 
> They have to. Anyone sharing a i915 dma-buf is susceptible to all kinds
> of (unprivileged) fun.
>  
> > Imo not even root should be allowed to break this, since it could put
> > drivers into a non-recoverable state. I think this must be restricted to
> > something known-unsafe-don't-enable-on-production like debugfs.
> 
> Providing fences is extremely useful, even for software buffers. (For
> the sake of argument, just imagine an asynchronous multithreaded llvmpipe
> wanting to support client fences for deferred rendering.) The only
> question in my mind is how much cotton wool to use.
> 
> > Other solutions which I don't like:
> > - Everyone needs to be able to recover. Given how much effort it is to
> >   just keep i915 hangcheck in working order I think that's totally
> >   illusionary to assume. At least once world+dog (atomic, v4l, ...) all
> >   consume/produce fences, subsystems where the usual assumption holds that
> >   async ops complete.
> > 
> > - Really long timeouts are allowed for root in vgem. Could lead to even
> >   more fun in testing i915 hangchecks I think, so don't like that much
> >   either.
> 
> The whole point is in testing our handling before we become suspectible
> to real world fail - because as you point out, not everyone guarantees
> that a fence will be signaled. I can't simply pass around i915 dma-buf
> simply because we may unwind them and in the process completely curtail
> being able to test a foriegn fence that hangs.

I think that's where we differ in opinion: Right now we do have the
guarantee that every fence gets signalled in finite time. For drivers
where that is not just guaranteed there must be a hangcheck to force the

Re: [Intel-gfx] [PATCH 1/5] drm/i915: unify first-stage engine struct setup

[Daniel Vetter]

On Wed, Jul 13, 2016 at 02:24:06PM +0100, Tvrtko Ursulin wrote:
> 
> On 13/07/16 14:16, Tvrtko Ursulin wrote:
> > 
> > On 13/07/16 13:23, Daniel Vetter wrote:
> > > On Fri, Jul 01, 2016 at 05:47:11PM +0100, Tvrtko Ursulin wrote:
> > > > From: Dave Gordon 
> > > > 
> > > > intel_lrc.c has a table of "logical rings" (meaning engines), while
> > > > intel_ringbuffer.c has separately open-coded initialisation for each
> > > > engine. We can deduplicate this somewhat by using the same first-stage
> > > > engine-setup function for both modes.
> > > > 
> > > > So here we expose the function that transfers information from the
> > > > static table of (all) known engines to the dev_priv->engine array of
> > > > engines available on this device (adjusting the names along the way)
> > > > and then embed calls to it in both the LRC and the legacy-mode setup.
> > > > 
> > > > Signed-off-by: Dave Gordon 
> > > > ---
> > > >   drivers/gpu/drm/i915/intel_lrc.c| 40
> > > > +
> > > >   drivers/gpu/drm/i915/intel_ringbuffer.c | 40
> > > > +
> > > >   drivers/gpu/drm/i915/intel_ringbuffer.h |  5 +
> > > >   3 files changed, 41 insertions(+), 44 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> > > > b/drivers/gpu/drm/i915/intel_lrc.c
> > > > index 339d8041075f..ed017f1a07a2 100644
> > > > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > > > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > > > @@ -1994,8 +1994,9 @@ logical_ring_default_vfuncs(struct
> > > > intel_engine_cs *engine)
> > > >   }
> > > > 
> > > >   static inline void
> > > > -logical_ring_default_irqs(struct intel_engine_cs *engine, unsigned
> > > > shift)
> > > > +logical_ring_default_irqs(struct intel_engine_cs *engine)
> > > >   {
> > > > +unsigned shift = engine->irq_shift;
> > > >   engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT << shift;
> > > >   engine->irq_keep_mask = GT_CONTEXT_SWITCH_INTERRUPT << shift;
> > > >   init_waitqueue_head(>irq_queue);
> > > > @@ -2096,14 +2097,14 @@ static int logical_render_ring_init(struct
> > > > intel_engine_cs *engine)
> > > >   return ret;
> > > >   }
> > > > 
> > > > -static const struct logical_ring_info {
> > > > +static const struct engine_info {
> > > >   const char *name;
> > > >   unsigned exec_id;
> > > >   unsigned guc_id;
> > > >   u32 mmio_base;
> > > >   unsigned irq_shift;
> > > >   int (*init)(struct intel_engine_cs *engine);
> > > > -} logical_rings[] = {
> > > > +} intel_engines[] = {
> > > >   [RCS] = {
> > > >   .name = "render ring",
> > > >   .exec_id = I915_EXEC_RENDER,
> > > > @@ -2146,20 +2147,31 @@ static const struct logical_ring_info {
> > > >   },
> > > >   };
> > > > 
> > > > -static struct intel_engine_cs *
> > > > -logical_ring_setup(struct drm_i915_private *dev_priv, enum
> > > > intel_engine_id id)
> > > > +struct intel_engine_cs *
> > > > +intel_engine_setup(struct drm_i915_private *dev_priv,
> > > > +   enum intel_engine_id id)
> > > 
> > > Kerneldoc for this would be nice. Also, we now have a mess between
> > > intel_lrc.c and intel_ringbuffer.c. Extracting intel_engine.c with the
> > 
> > Yes that was already suggested by Chris and I am trybotting the
> > additions today, again on his explicit request to progress this series.
> > 
> > > shared bits or something similar, plus cleanup up all the docs would be
> > > awesome as a follow up.
> > 
> > What do you mean "all the docs"? :D
> > 
> > > 
> > > With the kerneldoc added:
> > > 
> > > Reviewed-by: Daniel Vetter 
> > 
> > Thanks, will add.
> 
> Actually the function becomes static later in the series, when the common
> stuff is moved into intel_engine_cs.c.
> 
> Can I keep the r-b without adding kernel doc for it then?

Sure.
-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 3/6] drm/i915/huc: Add HuC fw loading support

[Daniel Vetter]

On Thu, Jul 14, 2016 at 03:08:41PM +0100, Dave Gordon wrote:
> On 13/07/16 13:48, Daniel Vetter wrote:
> > On Thu, Jun 23, 2016 at 02:52:41PM +0100, Peter Antoine wrote:
> > > On Thu, 23 Jun 2016, Dave Gordon wrote:
> > > > On 22/06/16 09:31, Daniel Vetter wrote:
> > > > No, the *correct* fix is to unify all the firmware loaders we have.
> > > > There should just be ONE piece of code that can be used to fetch and
> > > > load ANy firmware into ANY auxiliary microcontroller. NOT one per
> > > > microcontroller, all different -- that way lies madness.
> > > > 
> > > > We already had a unified loader for the HuC and GuC a year ago, but IIRC
> > > > the party line then was "just make it (GuC) specific, then copypaste it
> > > > for the second uC, and when we've got three versions we'll have learnt
> > > > how we really want a unified loader to behave."
> > > > 
> > > > Well. here's the copypaste, and we already have a different loader for
> > > > the DMC/CSR, so it must be time for (re-)unification.
> > > > 
> > > > .Dave.
> > > 
> > > Just to add, if you uc_fw_fetch() has an error code you will still have to
> > > remember the state of the fetch or at each reset/resume/etc... or you will
> > > have to try the firmware load again and that can take a long time. So the
> > > state will have to be re-instated.
> > > 
> > > Seeing this code was written with the given goals and were written in the
> > > same vane as code that was deemed acceptable, it seems weird at this late
> > > stage to change the design goals.
> > > 
> > > Note: this is the third time that these patches have been posted and were
> > > only rejected (as far as I know) due to no open-source user. Which there 
> > > is
> > > now, and is why I have reposted these patches.
> > 
> > I never liked the guc firmware code, but figure for one copy it's not
> > worth fighting over. Adding more copies (or perpetuating the design by
> > making it generic) isn't what I'm looking for.
> 
> *You* asked for more copies, back when we proposed a single unified solution
> last year. We already had a *single* GuC+HuC loader which could also have
> been extended to support the DMC as well, but at the time you wanted a
> GuC-specific version -- and by implication, a separate HuC loader -- *in
> addition to* the DMC loader.
> 
> > Firmware loading shouldn't be that complicated, really.
> 
> Maybe it shouldn't be, and maybe it isn't -- you may not be seeing how
> simple this code actually is. Fetch firmware, validate it, save it in a GEM
> object; later, DMA it to the h/w; at each stage keep track of status so we
> know what has been done and what is still to do (or redo, during reset).
> 
> Any complications are because the h/w (e.g. write-once memory) makes them
> necessary, or artefacts of the GEM object system, or because of the driver's
> byzantine sequence of operations during load/reset/suspend/resume/unload.
> 
> > The unified firmware loader is called request_firmware. If that's not good
> > enough, pls fix the core function, not paper code over in i915.
> 
> That's exactly the function we call. Then we have to validate and save the
> blob. And remember that we've done so.
> 
> > In that regard DMC/CSR is unified, everything else isn't yet.
> 
> Unified with what? Maybe the "DMC" is unified with the "CSR" -- which AFAIK
> are the same thing -- and the software just randomly uses both names to
> maximise confusion?
> 
>   if (HAS_CSR(dev)) {
>   struct intel_csr *csr = _priv->csr;
> 
>   err_printf(m, "DMC loaded: %s\n",
>  yesno(csr->dmc_payload != NULL));
>   err_printf(m, "DMC fw version: %d.%d\n",
>  CSR_VERSION_MAJOR(csr->version),
>  CSR_VERSION_MINOR(csr->version));
>   }
> ...
>   if (!IS_GEN9(dev_priv)) {
>   DRM_ERROR("No CSR support available for this platform\n");
>   return;
>   }
>   if (!dev_priv->csr.dmc_payload) {
>   DRM_ERROR("Tried to program CSR with empty payload\n");
>   return;
>   }
> 
> And according to the comments in intel_csr.c -- but not the code --
> 
> /*
>  * Firmware loading status will be one of the below states:
>  * FW_UNINITIALIZED, FW_LOADED, FW_FAILED.
>  *
>  * Once the firmware is written into the registers status will
>  * be moved from FW_UNINITIALIZED to FW_LOADED and for any
>  * erroneous condition status will be moved to FW_FAILED.
>  */
> 
> So I don't think you should hold this code up as a masterpiece of "unified"
> design -- which in any case you argued against last year, when we presented
> a unified loader. Specifically, you said, "In my experience trying to
> extract common code at all costs is harmful way too often."
> 
> Also, the approach taken in the DMC loader -- which appears to have been
> copypasted from a /very early/ version of the GuC loader, before I fixed the
> async-load problems -- just wouldn't work for the 

Re: [Intel-gfx] [PATCH 3/6] drm/i915/huc: Add HuC fw loading support

[Daniel Vetter]

On Wed, Jul 13, 2016 at 03:52:39PM +0100, Peter Antoine wrote:
> On Wed, 13 Jul 2016, Daniel Vetter wrote:
> 
> > On Thu, Jun 23, 2016 at 02:52:41PM +0100, Peter Antoine wrote:
> > > On Thu, 23 Jun 2016, Dave Gordon wrote:
> > > > On 22/06/16 09:31, Daniel Vetter wrote:
> > > > No, the *correct* fix is to unify all the firmware loaders we have.
> > > > There should just be ONE piece of code that can be used to fetch and
> > > > load ANy firmware into ANY auxiliary microcontroller. NOT one per
> > > > microcontroller, all different -- that way lies madness.
> > > > 
> > > > We already had a unified loader for the HuC and GuC a year ago, but IIRC
> > > > the party line then was "just make it (GuC) specific, then copypaste it
> > > > for the second uC, and when we've got three versions we'll have learnt
> > > > how we really want a unified loader to behave."
> > > > 
> > > > Well. here's the copypaste, and we already have a different loader for
> > > > the DMC/CSR, so it must be time for (re-)unification.
> > > > 
> > > > .Dave.
> > > > 
> > > 
> > > Just to add, if you uc_fw_fetch() has an error code you will still have to
> > > remember the state of the fetch or at each reset/resume/etc... or you will
> > > have to try the firmware load again and that can take a long time. So the
> > > state will have to be re-instated.
> > > 
> > > Seeing this code was written with the given goals and were written in the
> > > same vane as code that was deemed acceptable, it seems weird at this late
> > > stage to change the design goals.
> > > 
> > > Note: this is the third time that these patches have been posted and were
> > > only rejected (as far as I know) due to no open-source user. Which there 
> > > is
> > > now, and is why I have reposted these patches.
> > 
> > I never liked the guc firmware code, but figure for one copy it's not
> > worth fighting over. Adding more copies (or perpetuating the design by
> > making it generic) isn't what I'm looking for. Firmware loading shouldn't
> > be that complicated, really.
> > 
> > The unified firmware loader is called request_firmware. If that's not good
> > enough, pls fix the core function, not paper code over in i915. In that
> > regard DMC/CSR is unified, everything else isn't yet.
> > 
> > Iirc the big issue is delayed firmware loading for built-in i915 and fw
> > only available later on. This is an open issue in request_firmware() since
> > years, and there's various patches floating around. If the problem is that
> > Greg KH doesn't consider those patches, I can help with that. But not
> > pushing the core fix forward isn't acceptable imo. Once that fix is landed
> > we can treat request_firmware as reliable (it might take a while, hence
> > must be run in an async work like DMC loading), with no need to ever retry
> > anything. If fw loading fails we can just mark the entire render part of
> > the gpu as dead by injecting the equivalent of a non-recoverable hang
> > (async setup) or failing engine init with -EIO (if this is still
> > synchronous, which I don't expect really).
> > 
> > If there's another reason for this complexity, please explain since I'd
> > like to understand why we need this.
> > -Daniel
> > 
> 
> I was not involved at the start and I am porting code that others have
> written, but as far as I understand this, you requested that the code be
> duplicated. See Dave's comment above as he was involved.
> 
> The code uses request_firmware() to handle the load of the firmware into
> memory and the rest of this code manages the loading of that memory into the
> HuC's SRAM. This needs extra setup that should not really go into the
> generic firmware loader (one loader for uIA's make sense as this will
> futureproof the code). Also the SRAM is write-once memory and needs to be
> handled correctly. Also, the GuC needs to verify the HuC some this becomes a
> little be more fun.
> 
> Also, again you are ignoring the point that not having firmware is not
> fatal. We remember the state of the load as this is required to save time
> when we come out of reset to not waste time trying to reload the firmware,
> if it has failed already. They are other mechinums to do this, but they will
> always need some form of history.
> 
> Also, if request_firmware() is broken why has this not been fixed? If it has
> been broken for "years" why and how do you expect to to be fixed now? If the
> "not pushing the core fix is not acceptable" why has that not been done?

Apparently because I'm a too nice maintainer and allowed half-solutions to
get landed, under the expecations that people would indeed follow up and
fix things.

And yes, you care, you fix it, is how this works, whether you like it or
not.
-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

[Intel-gfx] [PATCH] drm/i915: Remove misleading CSR firmware loading docs

[Daniel Vetter]

I forgot to remove these when reworking the firmware loading sequence
last year. The new sequence is that we load firmware, and if it's not
there we entirely (and permanently) fail dmc setup.

Reported-by: Dave Gordon 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_csr.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index c3b33a10c15c..1ea0e1f43397 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -32,13 +32,6 @@
  * onwards to drive newly added DMC (Display microcontroller) in display
  * engine to save and restore the state of display engine when it enter into
  * low-power state and comes back to normal.
- *
- * Firmware loading status will be one of the below states: FW_UNINITIALIZED,
- * FW_LOADED, FW_FAILED.
- *
- * Once the firmware is written into the registers status will be moved from
- * FW_UNINITIALIZED to FW_LOADED and for any erroneous condition status will
- * be moved to FW_FAILED.
  */
 
 #define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin"
-- 
2.8.1

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

Re: [Intel-gfx] [PATCH] drm/i915: Move hangcheck reset from out of the engines into the GPU reset

[Chris Wilson]

On Thu, Jul 14, 2016 at 04:59:56PM +0300, Mika Kuoppala wrote:
> Chris Wilson  writes:
> 
> > Let's not reset the hangcheck as a side-effect of initialising the
> > engine, but as part of our GPU reset.
> >
> 
> I think TDR will need both the init and the reset.

It doesn't. This state is the purely global state for hangcheck. There's
probably a better spot, such as hangcheck expiration, but at the moment
this is better. TDR will be keeping its own data structures and should
not stop the global hangcheck from stepping in and saying enough is
enough.
-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 3/6] drm/i915/huc: Add HuC fw loading support

[Dave Gordon]

On 13/07/16 13:48, Daniel Vetter wrote:

On Thu, Jun 23, 2016 at 02:52:41PM +0100, Peter Antoine wrote:

On Thu, 23 Jun 2016, Dave Gordon wrote:

On 22/06/16 09:31, Daniel Vetter wrote:
No, the *correct* fix is to unify all the firmware loaders we have.
There should just be ONE piece of code that can be used to fetch and
load ANy firmware into ANY auxiliary microcontroller. NOT one per
microcontroller, all different -- that way lies madness.

We already had a unified loader for the HuC and GuC a year ago, but IIRC
the party line then was "just make it (GuC) specific, then copypaste it
for the second uC, and when we've got three versions we'll have learnt
how we really want a unified loader to behave."

Well. here's the copypaste, and we already have a different loader for
the DMC/CSR, so it must be time for (re-)unification.

.Dave.


Just to add, if you uc_fw_fetch() has an error code you will still have to
remember the state of the fetch or at each reset/resume/etc... or you will
have to try the firmware load again and that can take a long time. So the
state will have to be re-instated.

Seeing this code was written with the given goals and were written in the
same vane as code that was deemed acceptable, it seems weird at this late
stage to change the design goals.

Note: this is the third time that these patches have been posted and were
only rejected (as far as I know) due to no open-source user. Which there is
now, and is why I have reposted these patches.


I never liked the guc firmware code, but figure for one copy it's not
worth fighting over. Adding more copies (or perpetuating the design by
making it generic) isn't what I'm looking for.


*You* asked for more copies, back when we proposed a single unified 
solution last year. We already had a *single* GuC+HuC loader which could 
also have been extended to support the DMC as well, but at the time you 
wanted a GuC-specific version -- and by implication, a separate HuC 
loader -- *in addition to* the DMC loader.


> Firmware loading shouldn't be that complicated, really.

Maybe it shouldn't be, and maybe it isn't -- you may not be seeing how 
simple this code actually is. Fetch firmware, validate it, save it in a 
GEM object; later, DMA it to the h/w; at each stage keep track of status 
so we know what has been done and what is still to do (or redo, during 
reset).


Any complications are because the h/w (e.g. write-once memory) makes 
them necessary, or artefacts of the GEM object system, or because of the 
driver's byzantine sequence of operations during 
load/reset/suspend/resume/unload.



The unified firmware loader is called request_firmware. If that's not good
enough, pls fix the core function, not paper code over in i915.


That's exactly the function we call. Then we have to validate and save 
the blob. And remember that we've done so.



In that regard DMC/CSR is unified, everything else isn't yet.


Unified with what? Maybe the "DMC" is unified with the "CSR" -- which 
AFAIK are the same thing -- and the software just randomly uses both 
names to maximise confusion?


if (HAS_CSR(dev)) {
struct intel_csr *csr = _priv->csr;

err_printf(m, "DMC loaded: %s\n",
   yesno(csr->dmc_payload != NULL));
err_printf(m, "DMC fw version: %d.%d\n",
   CSR_VERSION_MAJOR(csr->version),
   CSR_VERSION_MINOR(csr->version));
}
...
if (!IS_GEN9(dev_priv)) {
DRM_ERROR("No CSR support available for this platform\n");
return;
}
if (!dev_priv->csr.dmc_payload) {
DRM_ERROR("Tried to program CSR with empty payload\n");
return;
}

And according to the comments in intel_csr.c -- but not the code --

/*
 * Firmware loading status will be one of the below states:
 * FW_UNINITIALIZED, FW_LOADED, FW_FAILED.
 *
 * Once the firmware is written into the registers status will
 * be moved from FW_UNINITIALIZED to FW_LOADED and for any
 * erroneous condition status will be moved to FW_FAILED.
 */

So I don't think you should hold this code up as a masterpiece of 
"unified" design -- which in any case you argued against last year, when 
we presented a unified loader. Specifically, you said, "In my experience 
trying to extract common code at all costs is harmful way too often."


Also, the approach taken in the DMC loader -- which appears to have been 
copypasted from a /very early/ version of the GuC loader, before I fixed 
the async-load problems -- just wouldn't work for the HuC/GuC, where the 
kernel needs to know when the firmware load has been completed so that 
it can start sending work to the GuC. The DMC loader only works because 
it doesn't actually matter when (or if) it's loaded. It would be 
*completely wrong* to load the HuC/Guc that way.



Iirc the big issue is delayed firmware loading for built-in i915 and fw
only available 

Re: [Intel-gfx] [PATCH 7/8] drm/i915: Defer enabling rc6 til after we submit the first batch/context

[Chris Wilson]

On Thu, Jul 14, 2016 at 04:56:50PM +0300, Mika Kuoppala wrote:
> Chris Wilson  writes:
> 
> > Some hardware requires a valid render context before it can initiate
> > rc6 power gating of the GPU; the default state of the GPU is not
> > sufficient and may lead to undefined behaviour. The first execution of
> > any batch will load the "golden render state", at which point it is safe
> > to enable rc6. As we do not forcibly load the kernel context at resume,
> > we have to hook into the batch submission to be sure that the render
> > state is setup before enabling rc6.
> >
> > However, since we don't enable powersaving until that first batch, we
> > queued a delayed task in order to guarantee that the batch is indeed
> > submitted.
> >
> > v2: Rearrange intel_disable_gt_powersave() to match.
> > v3: Apply user specified cur_freq (or idle_freq if not set).
> > v4: Give in, and supply a delayed work to autoenable rc6
> > v5: Mika suggested a couple of better names for delayed_resume_work
> > v6: Rebalance rpm_put around the autoenable task
> >
> > Signed-off-by: Chris Wilson 
> > Cc: Mika Kuoppala 
> > Cc: Ville Syrjälä 
> 
> Ta for splitting stuff out from this patch(set) to smaller
> bits.

It was only in the other because I was waiting for you to come back from
holiday! (And trying to progress the other means sending all patches up
to that point so that CI has a hope of sanity checking them.)
-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

[Intel-gfx] [CI resend 1/2] drm/i915: compile-time consistency check on __EXEC_OBJECT flags

[Dave Gordon]

Two different sets of flag bits are stored in the 'flags' member of a
'struct drm_i915_gem_exec_object2', and they're defined in two different
source files, increasing the risk of an accidental clash.

Some flags in this field are supplied by the user; these are defined in
i915_drm.h, and they start from the LSB and work up.

Other flags are defined in i915_gem_execbuffer, for internal use within
that file only; they start from the MSB and work down.

So here we add a compile-time check that the two sets of flags do not
overlap, which would cause all sorts of confusion.

Signed-off-by: Dave Gordon 
Reviewed-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 12 
 include/uapi/drm/i915_drm.h| 11 ++-
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 1978633..1bb1f25 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -34,10 +34,11 @@
 #include 
 #include 
 
-#define  __EXEC_OBJECT_HAS_PIN (1<<31)
-#define  __EXEC_OBJECT_HAS_FENCE (1<<30)
-#define  __EXEC_OBJECT_NEEDS_MAP (1<<29)
-#define  __EXEC_OBJECT_NEEDS_BIAS (1<<28)
+#define  __EXEC_OBJECT_HAS_PIN (1<<31)
+#define  __EXEC_OBJECT_HAS_FENCE   (1<<30)
+#define  __EXEC_OBJECT_NEEDS_MAP   (1<<29)
+#define  __EXEC_OBJECT_NEEDS_BIAS  (1<<28)
+#define  __EXEC_OBJECT_INTERNAL_FLAGS (0xf<<28) /* all of the above */
 
 #define BATCH_OFFSET_BIAS (256*1024)
 
@@ -1007,6 +1008,9 @@ static bool only_mappable_for_reloc(unsigned int flags)
unsigned invalid_flags;
int i;
 
+   /* INTERNAL flags must not overlap with external ones */
+   BUILD_BUG_ON(__EXEC_OBJECT_INTERNAL_FLAGS & 
~__EXEC_OBJECT_UNKNOWN_FLAGS);
+
invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS;
if (USES_FULL_PPGTT(dev))
invalid_flags |= EXEC_OBJECT_NEEDS_GTT;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index d7e81a3..51b9360 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -698,12 +698,13 @@ struct drm_i915_gem_exec_object2 {
 */
__u64 offset;
 
-#define EXEC_OBJECT_NEEDS_FENCE (1<<0)
-#define EXEC_OBJECT_NEEDS_GTT  (1<<1)
-#define EXEC_OBJECT_WRITE  (1<<2)
+#define EXEC_OBJECT_NEEDS_FENCE (1<<0)
+#define EXEC_OBJECT_NEEDS_GTT   (1<<1)
+#define EXEC_OBJECT_WRITE   (1<<2)
 #define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3)
-#define EXEC_OBJECT_PINNED (1<<4)
-#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_PINNED<<1)
+#define EXEC_OBJECT_PINNED  (1<<4)
+/* All remaining bits are MBZ and RESERVED FOR FUTURE USE */
+#define __EXEC_OBJECT_UNKNOWN_FLAGS(-(EXEC_OBJECT_PINNED<<1))
__u64 flags;
 
__u64 rsvd1;
-- 
1.9.1

___
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: refactor eb_get_batch()

[Chris Wilson]

On Thu, Jul 14, 2016 at 02:12:55PM +0100, Dave Gordon wrote:
> On 13/07/16 13:44, Chris Wilson wrote:
> >On Wed, Jul 13, 2016 at 02:38:16PM +0200, Daniel Vetter wrote:
> >>On Thu, Jun 30, 2016 at 04:12:49PM +0100, Dave Gordon wrote:
> >>>Precursor for fix to secure batch execution. We will need to be able to
> >>>retrieve the batch VMA (as well as the batch itself) from the eb list,
> >>>so this patch extracts that part of eb_get_batch() into a separate
> >>>function, and moves both parts to a more logical place in the file, near
> >>>where the eb list is created.
> >>>
> >>>Also, it may not be obvious, but the current execbuffer2 ioctl interface
> >>>requires that the buffer object containing the batch-to-be-executed be
> >>>the LAST entry in the exec2_list[] array (I expected it to be the first!).
> >>>
> >>>To clarify this, we can replace the rather obscure construct
> >>>   "list_entry(eb->vmas.prev, ...)"
> >>>in the old version of eb_get_batch() with the equivalent but more explicit
> >>>   "list_last_entry(>vmas,...)"
> >>>in the new eb_get_batch_vma() and of course add an explanatory comment.
> >>>
> >>>Signed-off-by: Dave Gordon 
> >>
> >>I have no context on the secure batch fix you're talking about, but this
> >>here makes sense as an independent cleanup.
> >
> >It won't help though, so this is just churn for no purpose.
> >-Chris
> 
> At the very least, it replaces a confusing construct with
> a comprehensible one annotated with an explanatory comment.

No. It deepens a confusion in the code that I've been trying to get
removed over the last couple of years.
 
> Separating finding the VMA for the batch from finding the batch itself
> also improves clarity and costs nothing (compiler inlines it anyway).

No. That's the confusion you have here. The object is irrelevant.
 
> Comprehensibility -- and hence maintainability -- is always
> a worthwhile purpose :)

s/comprehensibility/greater confusion/

> BTW, do the comments in this code from patch
> 
> d23db88 drm/i915: Prevent negative relocation deltas from wrapping
> 
> still apply? 'Cos I think it's pretty ugly to be setting a flag
> on a VMA as a side-effect of a "lookup" type operation :( Surely
> cleaner to do that sort of think at the top level i.e. inside
> i915_gem_do_execbuffer() ?

The comment is wrong since the practice is more widespread and it is
a particular hw bug on Ivybridge.
-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: Move hangcheck reset from out of the engines into the GPU reset

[Mika Kuoppala]

Chris Wilson  writes:

> Let's not reset the hangcheck as a side-effect of initialising the
> engine, but as part of our GPU reset.
>

I think TDR will need both the init and the reset.

-Mika

> Signed-off-by: Chris Wilson 
> Cc: Mika Kuoppala 
> Cc: Arun Siluvery 
> Cc: Tvrtko Ursulin 
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 1 +
>  drivers/gpu/drm/i915/intel_lrc.c| 2 --
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 2 --
>  3 files changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index b788f97cd4cf..978fa6f6f747 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3169,6 +3169,7 @@ static void i915_gem_reset_engine_cleanup(struct 
> intel_engine_cs *engine)
>   }
>  
>   intel_ring_init_seqno(engine, engine->last_submitted_seqno);
> + intel_engine_init_hangcheck(engine);
>  }
>  
>  void i915_gem_reset(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> b/drivers/gpu/drm/i915/intel_lrc.c
> index b6af635e3a0f..377cfbfb71fd 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1529,8 +1529,6 @@ static int gen8_init_common_ring(struct intel_engine_cs 
> *engine)
>   engine->next_context_status_buffer = next_context_status_buffer_hw;
>   DRM_DEBUG_DRIVER("Execlists enabled for %s\n", engine->name);
>  
> - intel_engine_init_hangcheck(engine);
> -
>   return intel_mocs_init_engine(engine);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 94c8ef461721..8a9e035341a9 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -627,8 +627,6 @@ static int init_ring_common(struct intel_engine_cs 
> *engine)
>   ringbuf->tail = I915_READ_TAIL(engine) & TAIL_ADDR;
>   intel_ring_update_space(ringbuf);
>  
> - intel_engine_init_hangcheck(engine);
> -
>  out:
>   intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>  
> -- 
> 2.8.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 7/8] drm/i915: Defer enabling rc6 til after we submit the first batch/context

[Mika Kuoppala]

Chris Wilson  writes:

> Some hardware requires a valid render context before it can initiate
> rc6 power gating of the GPU; the default state of the GPU is not
> sufficient and may lead to undefined behaviour. The first execution of
> any batch will load the "golden render state", at which point it is safe
> to enable rc6. As we do not forcibly load the kernel context at resume,
> we have to hook into the batch submission to be sure that the render
> state is setup before enabling rc6.
>
> However, since we don't enable powersaving until that first batch, we
> queued a delayed task in order to guarantee that the batch is indeed
> submitted.
>
> v2: Rearrange intel_disable_gt_powersave() to match.
> v3: Apply user specified cur_freq (or idle_freq if not set).
> v4: Give in, and supply a delayed work to autoenable rc6
> v5: Mika suggested a couple of better names for delayed_resume_work
> v6: Rebalance rpm_put around the autoenable task
>
> Signed-off-by: Chris Wilson 
> Cc: Mika Kuoppala 
> Cc: Ville Syrjälä 

Ta for splitting stuff out from this patch(set) to smaller
bits. I think it's all done now.

Reviewed-by: Mika Kuoppala  ---
>  drivers/gpu/drm/i915/i915_drv.c  |  11 +--
>  drivers/gpu/drm/i915/i915_drv.h  |   2 +-
>  drivers/gpu/drm/i915/i915_gem.c  |   3 +
>  drivers/gpu/drm/i915/intel_display.c |   1 -
>  drivers/gpu/drm/i915/intel_drv.h |   3 +-
>  drivers/gpu/drm/i915/intel_pm.c  | 136 
> +--
>  drivers/gpu/drm/i915/intel_uncore.c  |   2 +-
>  7 files changed, 94 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b9a811750ca8..c6cc01faaa36 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1343,7 +1343,7 @@ void i915_driver_unload(struct drm_device *dev)
>   i915_destroy_error_state(dev);
>  
>   /* Flush any outstanding unpin_work. */
> - flush_workqueue(dev_priv->wq);
> + drain_workqueue(dev_priv->wq);
>  
>   intel_guc_fini(dev);
>   i915_gem_fini(dev);
> @@ -1458,8 +1458,6 @@ static int i915_drm_suspend(struct drm_device *dev)
>  
>   intel_guc_suspend(dev);
>  
> - intel_suspend_gt_powersave(dev_priv);
> -
>   intel_display_suspend(dev);
>  
>   intel_dp_mst_suspend(dev);
> @@ -1652,6 +1650,7 @@ static int i915_drm_resume(struct drm_device *dev)
>  
>   intel_opregion_notify_adapter(dev_priv, PCI_D0);
>  
> + intel_autoenable_gt_powersave(dev_priv);
>   drm_kms_helper_poll_enable(dev);
>  
>   enable_rpm_wakeref_asserts(dev_priv);
> @@ -1778,8 +1777,6 @@ int i915_reset(struct drm_i915_private *dev_priv)
>   unsigned reset_counter;
>   int ret;
>  
> - intel_reset_gt_powersave(dev_priv);
> -
>   mutex_lock(>struct_mutex);
>  
>   /* Clear any previous failed attempts at recovery. Time to try again. */
> @@ -1835,8 +1832,7 @@ int i915_reset(struct drm_i915_private *dev_priv)
>* previous concerns that it doesn't respond well to some forms
>* of re-init after reset.
>*/
> - if (INTEL_INFO(dev)->gen > 5)
> - intel_enable_gt_powersave(dev_priv);
> + intel_autoenable_gt_powersave(dev_priv);
>  
>   return 0;
>  
> @@ -2459,7 +2455,6 @@ static int intel_runtime_resume(struct device *device)
>* we can do is to hope that things will still work (and disable RPM).
>*/
>   i915_gem_init_swizzling(dev);
> - gen6_update_ring_freq(dev_priv);
>  
>   intel_runtime_pm_enable_interrupts(dev_priv);
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index fe85235367be..1fdca3bb7f33 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1188,7 +1188,7 @@ struct intel_gen6_power_mgmt {
>   bool client_boost;
>  
>   bool enabled;
> - struct delayed_work delayed_resume_work;
> + struct delayed_work autoenable_work;
>   unsigned boosts;
>  
>   struct intel_rps_client semaphores, mmioflips;
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9d8c26f42dee..67c3bffb700d 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2842,6 +2842,7 @@ static void i915_gem_mark_busy(const struct 
> intel_engine_cs *engine)
>   intel_runtime_pm_get_noresume(dev_priv);
>   dev_priv->gt.awake = true;
>  
> + intel_enable_gt_powersave(dev_priv);
>   i915_update_gfx_val(dev_priv);
>   if (INTEL_GEN(dev_priv) >= 6)
>   gen6_rps_busy(dev_priv);
> @@ -4980,6 +4981,8 @@ i915_gem_suspend(struct drm_device *dev)
>   struct drm_i915_private *dev_priv = to_i915(dev);
>   int ret = 0;
>  
> + intel_suspend_gt_powersave(dev_priv);
> +
>   mutex_lock(>struct_mutex);
>   ret = 

[Intel-gfx] [CI resend 2/2] drm/i915: refactor eb_get_batch()

[Dave Gordon]

Precursor for fix to secure batch execution. We will need to be able to
retrieve the batch VMA (as well as the batch itself) from the eb list,
so this patch extracts that part of eb_get_batch() into a separate
function, and moves both parts to a more logical place in the file, near
where the eb list is created.

Also, it may not be obvious, but the current execbuffer2 ioctl interface
requires that the buffer object containing the batch-to-be-executed be
the LAST entry in the exec2_list[] array (I expected it to be the
first!).

To clarify this, we can replace the rather obscure construct
"list_entry(eb->vmas.prev, ...)"
in the old version of eb_get_batch() with the equivalent but more
explicit
"list_last_entry(>vmas,...)"
in the new eb_get_batch_vma() and of course add an explanatory comment.

Signed-off-by: Dave Gordon 
Reviewed-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 49 ++
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 1bb1f25..f6724ae 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -186,6 +186,35 @@ struct eb_vmas {
return ret;
 }
 
+static inline struct i915_vma *
+eb_get_batch_vma(struct eb_vmas *eb)
+{
+   /* The batch is always the LAST item in the VMA list */
+   struct i915_vma *vma = list_last_entry(>vmas, typeof(*vma), 
exec_list);
+
+   return vma;
+}
+
+static struct drm_i915_gem_object *
+eb_get_batch(struct eb_vmas *eb)
+{
+   struct i915_vma *vma = eb_get_batch_vma(eb);
+
+   /*
+* SNA is doing fancy tricks with compressing batch buffers, which leads
+* to negative relocation deltas. Usually that works out ok since the
+* relocate address is still positive, except when the batch is placed
+* very low in the GTT. Ensure this doesn't happen.
+*
+* Note that actual hangs have only been observed on gen7, but for
+* paranoia do it everywhere.
+*/
+   if ((vma->exec_entry->flags & EXEC_OBJECT_PINNED) == 0)
+   vma->exec_entry->flags |= __EXEC_OBJECT_NEEDS_BIAS;
+
+   return vma->obj;
+}
+
 static struct i915_vma *eb_get_vma(struct eb_vmas *eb, unsigned long handle)
 {
if (eb->and < 0) {
@@ -1341,26 +1370,6 @@ static bool only_mappable_for_reloc(unsigned int flags)
return file_priv->bsd_ring;
 }
 
-static struct drm_i915_gem_object *
-eb_get_batch(struct eb_vmas *eb)
-{
-   struct i915_vma *vma = list_entry(eb->vmas.prev, typeof(*vma), 
exec_list);
-
-   /*
-* SNA is doing fancy tricks with compressing batch buffers, which leads
-* to negative relocation deltas. Usually that works out ok since the
-* relocate address is still positive, except when the batch is placed
-* very low in the GTT. Ensure this doesn't happen.
-*
-* Note that actual hangs have only been observed on gen7, but for
-* paranoia do it everywhere.
-*/
-   if ((vma->exec_entry->flags & EXEC_OBJECT_PINNED) == 0)
-   vma->exec_entry->flags |= __EXEC_OBJECT_NEEDS_BIAS;
-
-   return vma->obj;
-}
-
 #define I915_USER_RINGS (4)
 
 static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = {
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH v3] drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl)

[Chris Wilson]

On Thu, Jul 14, 2016 at 02:23:04PM +0100, Chris Wilson wrote:
> The biggest reason I had against going the sw_sync only route was that
> vgem should provide unprivileged fences and that through the bookkeeping
> in vgem we can keep them safe, ensure that we don't leak random buffers
> or fences. (And I need a source of foriegn dma-buf with implicit fence
> tracking with which I can try and break the driver.)

And for testing passing around content + fences is more useful than
passing fences alone.
-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] ✗ Ro.CI.BAT: warning for drm/i915/guc: Protect against HAS_GUC_* returning true values other than one (rev4)

[Dave Gordon]

On 13/07/16 14:38, Patchwork wrote:

== Series Details ==

Series: drm/i915/guc: Protect against HAS_GUC_* returning true values other 
than one (rev4)
URL   : https://patchwork.freedesktop.org/series/9473/
State : warning

== Summary ==

Series 9473v4 drm/i915/guc: Protect against HAS_GUC_* returning true values 
other than one
http://patchwork.freedesktop.org/api/1.0/series/9473/revisions/4/mbox

Test gem_exec_suspend:
 Subgroup basic-s3:
 pass   -> DMESG-WARN (ro-bdw-i7-5557U)
Test kms_pipe_crc_basic:
 Subgroup suspend-read-crc-pipe-a:
 dmesg-warn -> PASS   (ro-skl3-i5-6260u)
 skip   -> DMESG-WARN (ro-bdw-i7-5557U)


Both of these look like
https://bugs.freedesktop.org/show_bug.cgi?id=96614
[BAT BDW] *ERROR* failed to enable link training/failed to start channel 
equalization


.Dave.


fi-kbl-qkkr  total:237  pass:174  dwarn:27  dfail:2   fail:7   skip:27
fi-skl-i5-6260u  total:237  pass:189  dwarn:27  dfail:2   fail:7   skip:12
fi-skl-i7-6700k  total:237  pass:200  dwarn:2   dfail:0   fail:9   skip:26
ro-bdw-i5-5250u  total:237  pass:210  dwarn:2   dfail:0   fail:9   skip:16
ro-bdw-i7-5557U  total:237  pass:210  dwarn:3   dfail:0   fail:9   skip:15
ro-bsw-n3050 total:217  pass:170  dwarn:0   dfail:0   fail:4   skip:42
ro-skl3-i5-6260u total:237  pass:213  dwarn:3   dfail:0   fail:9   skip:12
fi-snb-i7-2600 failed to connect after reboot
ro-bdw-i7-5600u failed to connect after reboot
ro-byt-n2820 failed to connect after reboot
ro-hsw-i3-4010u failed to connect after reboot
ro-hsw-i7-4770r failed to connect after reboot
ro-ilk1-i5-650 failed to connect after reboot
ro-ilk-i7-620lm failed to connect after reboot
ro-ivb-i7-3770 failed to connect after reboot
ro-snb-i7-2620M failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1483/

e8b1f0b drm-intel-nightly: 2016y-07m-13d-10h-57m-19s UTC integration manifest
40c374e drm/i915/guc: symbolic names for user load/submission preferences



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

Re: [Intel-gfx] [PATCH v3] drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl)

[Chris Wilson]

On Thu, Jul 14, 2016 at 02:40:59PM +0200, Daniel Vetter wrote:
> On Thu, Jul 14, 2016 at 11:11:02AM +0100, Chris Wilson wrote:
> > On Thu, Jul 14, 2016 at 10:59:04AM +0100, Chris Wilson wrote:
> > > On Thu, Jul 14, 2016 at 10:12:17AM +0200, Daniel Vetter wrote:
> > > > On Thu, Jul 14, 2016 at 08:04:19AM +0100, Chris Wilson wrote:
> > > > > vGEM buffers are useful for passing data between software clients and
> > > > > hardware renders. By allowing the user to create and attach fences to
> > > > > the exported vGEM buffers (on the dma-buf), the user can implement a
> > > > > deferred renderer and queue hardware operations like flipping and then
> > > > > signal the buffer readiness (i.e. this allows the user to schedule
> > > > > operations out-of-order, but have them complete in-order).
> > > > > 
> > > > > This also makes it much easier to write tightly controlled testcases 
> > > > > for
> > > > > dma-buf fencing and signaling between hardware drivers.
> > > > > 
> > > > > v2: Don't pretend the fences exist in an ordered timeline, but 
> > > > > allocate
> > > > > a separate fence-context for each fence so that the fences are
> > > > > unordered.
> > > > > v3: Make the debug output more interesting, and so the signaled 
> > > > > status.
> > > > > 
> > > > > Testcase: igt/vgem_basic/dmabuf-fence
> > > > > Signed-off-by: Chris Wilson 
> > > > > Cc: Sean Paul 
> > > > > Cc: Zach Reizner 
> > > > > Cc: Gustavo Padovan 
> > > > > Cc: Daniel Vetter 
> > > > > Acked-by: Zach Reizner 
> > > > 
> > > > One thing I completely forgotten: This allows userspace to hang kernel
> > > > drivers. i915 (and other gpu drivers) can recover using hangcheck, but
> > > > dumber drivers (v4l, if that ever happens) probably never except such a
> > > > case. We've had a similar discusion with the userspace fences exposed in
> > > > sw_fence, and decided to move all those ioctl into debugfs. I think we
> > > > should do the same for this vgem-based debugging of implicit sync. Sorry
> > > > for realizing this this late.
> > > 
> > > One of the very tests I make is to ensure that we recover from such a
> > > hang. I don't see the difference between this any of the other ways
> > > userspace can shoot itself (and others) in the foot.
> > 
> > So one solution would be to make vgem fences automatically timeout (with
> > a flag for root to override for the sake of testing hang detection).
> 
> The problem is other drivers. E.g. right now atomic helpers assume that
> fences will signal, and can't recover if they don't. This is why drivers
> where things might fail must have some recovery (hangcheck, timeout) to
> make sure dma_fences always signal.

Urm, all the atomic helpers should work with fails. The waits on dma-buf
should be before any hardware is modified and so cancellation is trivial.
Anyone using a foriegn fence (or even native) must cope that it may not
meet some deadline.

They have to. Anyone sharing a i915 dma-buf is susceptible to all kinds
of (unprivileged) fun.
 
> Imo not even root should be allowed to break this, since it could put
> drivers into a non-recoverable state. I think this must be restricted to
> something known-unsafe-don't-enable-on-production like debugfs.

Providing fences is extremely useful, even for software buffers. (For
the sake of argument, just imagine an asynchronous multithreaded llvmpipe
wanting to support client fences for deferred rendering.) The only
question in my mind is how much cotton wool to use.

> Other solutions which I don't like:
> - Everyone needs to be able to recover. Given how much effort it is to
>   just keep i915 hangcheck in working order I think that's totally
>   illusionary to assume. At least once world+dog (atomic, v4l, ...) all
>   consume/produce fences, subsystems where the usual assumption holds that
>   async ops complete.
> 
> - Really long timeouts are allowed for root in vgem. Could lead to even
>   more fun in testing i915 hangchecks I think, so don't like that much
>   either.

The whole point is in testing our handling before we become suspectible
to real world fail - because as you point out, not everyone guarantees
that a fence will be signaled. I can't simply pass around i915 dma-buf
simply because we may unwind them and in the process completely curtail
being able to test a foriegn fence that hangs.

> I think the best option is to just do the same as we've done for sw_fence,
> and move it to debugfs. We could reuse the debugfs sw_fence interface to
> create them (gives us more control as a bonus), and just have an ioctl to
> attach fences to vgem (which could be unpriviledged).

The biggest reason I had against going the sw_sync only route was that
vgem should provide unprivileged fences and that through the bookkeeping
in vgem we can keep them safe, ensure that we don't leak random buffers
or fences. (And I 

Re: [Intel-gfx] [PATCH] Revert "drm: Resurrect atomic rmfb code"

[Daniel Vetter]

On Thu, Jul 14, 2016 at 03:16:34PM +0200, Daniel Vetter wrote:
> This reverts commit 11c21e73f848844d439cbccb42a1018b8c560e5c.
> 
> For reasons totally unclear this manages to wreak havoc with the audio
> rpm refcount:
> 
> [ cut here ]
> WARNING: CPU: 0 PID: 215 at drivers/gpu/drm/i915/intel_runtime_pm.c:1729 
> intel_display_power_put+0xe8/0x100 [i915]
> Use count on domain AUDIO is already zero
> Modules linked in: i915 ax88179_178a usbnet mii snd_hda_codec_hdmi 
> snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec 
> x86_pkg_temp_thermal snd_hwdep intel_powerclamp snd_hda_core co
> f_pclmul crc32_pclmul snd_pcm ghash_clmulni_intel mei_me mei e1000e ptp 
> pps_core i2c_hid [last unloaded: i915]
> CPU: 0 PID: 215 Comm: kworker/0:2 Not tainted 4.7.0-rc6+ #44
> Hardware name: Intel Corporation Skylake Client platform/Skylake Halo DDR4 
> RVP11, BIOS SKLSE2R1.R00.X106.B00.1601180206 01/18/2016
> Workqueue: events output_poll_execute
>   88045573fa38 813a2d6b 88045573fa88
>   88045573fa78 81075db6 06c15a59
>  88045a59a238 88045a590054 88045a59 88045a59
> Call Trace:
>  [] dump_stack+0x4d/0x72
>  [] __warn+0xc6/0xe0
>  [] warn_slowpath_fmt+0x4a/0x50
>  [] ? hsw_audio_codec_disable+0xdd/0x110 [i915]
>  [] intel_display_power_put+0xe8/0x100 [i915]
>  [] intel_disable_ddi+0x46/0x80 [i915]
>  [] haswell_crtc_disable+0x16f/0x290 [i915]
>  [] intel_atomic_commit_tail+0x153/0x10e0 [i915]
>  [] ? drm_atomic_helper_swap_state+0x140/0x2d0
>  [] intel_atomic_commit+0x3fd/0x520 [i915]
>  [] ? drm_atomic_add_affected_connectors+0x22/0xf0
>  [] drm_atomic_commit+0x32/0x50
>  [] restore_fbdev_mode+0x147/0x260
>  [] drm_fb_helper_restore_fbdev_mode_unlocked+0x2e/0x70
>  [] drm_fb_helper_set_par+0x28/0x50
>  [] drm_fb_helper_hotplug_event+0x143/0x180
>  [] intel_fbdev_output_poll_changed+0x15/0x20 [i915]
>  [] drm_kms_helper_hotplug_event+0x22/0x30
>  [] output_poll_execute+0x192/0x1e0
>  [] process_one_work+0x14c/0x480
>  [] worker_thread+0x24a/0x4e0
>  [] ? process_one_work+0x480/0x480
>  [] ? process_one_work+0x480/0x480
>  [] kthread+0xc4/0xe0
>  [] ret_from_fork+0x1f/0x40
>  [] ? kthread_worker_fn+0x180/0x180
> ---[ end trace 2d440da5f0c053e4 ]---
> 
> Instead of scratching heads too much while CI is down, let's revert
> before more trouble is caused.
> 
> Cc: Maarten Lankhorst 
> Cc: Mika Kuoppala 
> Cc: Ville Syrjälä 
> Reported-by: Mika Kuoppala 
> Reported-by: Ville Syrjälä 
> Acked-by: Mika Kuoppala 
> Acked-by: Ville Syrjälä 
> Signed-off-by: Daniel Vetter 

... and applied.
-Daniel

> ---
>  drivers/gpu/drm/drm_atomic.c| 66 
> -
>  drivers/gpu/drm/drm_crtc.c  |  6 
>  drivers/gpu/drm/drm_crtc_internal.h |  1 -
>  3 files changed, 73 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 34cdbe154540..6712278c73f0 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1590,72 +1590,6 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
>  }
>  EXPORT_SYMBOL(drm_atomic_clean_old_fb);
>  
> -int drm_atomic_remove_fb(struct drm_framebuffer *fb)
> -{
> - struct drm_modeset_acquire_ctx ctx;
> - struct drm_device *dev = fb->dev;
> - struct drm_atomic_state *state;
> - struct drm_plane *plane;
> - int ret = 0;
> - unsigned plane_mask;
> -
> - state = drm_atomic_state_alloc(dev);
> - if (!state)
> - return -ENOMEM;
> -
> - drm_modeset_acquire_init(, 0);
> - state->acquire_ctx = 
> -
> -retry:
> - plane_mask = 0;
> - ret = drm_modeset_lock_all_ctx(dev, );
> - if (ret)
> - goto unlock;
> -
> - drm_for_each_plane(plane, dev) {
> - struct drm_plane_state *plane_state;
> -
> - if (plane->state->fb != fb)
> - continue;
> -
> - plane_state = drm_atomic_get_plane_state(state, plane);
> - if (IS_ERR(plane_state)) {
> - ret = PTR_ERR(plane_state);
> - goto unlock;
> - }
> -
> - drm_atomic_set_fb_for_plane(plane_state, NULL);
> - ret = drm_atomic_set_crtc_for_plane(plane_state, NULL);
> - if (ret)
> - goto unlock;
> -
> - plane_mask |= BIT(drm_plane_index(plane));
> -
> - plane->old_fb = plane->fb;
> - plane->fb = NULL;
> - }
> -
> - if (plane_mask)
> - ret = drm_atomic_commit(state);
> -
> -unlock:
> - if (plane_mask)
> - drm_atomic_clean_old_fb(dev, plane_mask, ret);
> -
> - if (ret == -EDEADLK) {
> - 

[Intel-gfx] [PATCH] Revert "drm: Resurrect atomic rmfb code"

[Daniel Vetter]

This reverts commit 11c21e73f848844d439cbccb42a1018b8c560e5c.

For reasons totally unclear this manages to wreak havoc with the audio
rpm refcount:

[ cut here ]
WARNING: CPU: 0 PID: 215 at drivers/gpu/drm/i915/intel_runtime_pm.c:1729 
intel_display_power_put+0xe8/0x100 [i915]
Use count on domain AUDIO is already zero
Modules linked in: i915 ax88179_178a usbnet mii snd_hda_codec_hdmi 
snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec x86_pkg_temp_thermal 
snd_hwdep intel_powerclamp snd_hda_core co
f_pclmul crc32_pclmul snd_pcm ghash_clmulni_intel mei_me mei e1000e ptp 
pps_core i2c_hid [last unloaded: i915]
CPU: 0 PID: 215 Comm: kworker/0:2 Not tainted 4.7.0-rc6+ #44
Hardware name: Intel Corporation Skylake Client platform/Skylake Halo DDR4 
RVP11, BIOS SKLSE2R1.R00.X106.B00.1601180206 01/18/2016
Workqueue: events output_poll_execute
  88045573fa38 813a2d6b 88045573fa88
  88045573fa78 81075db6 06c15a59
 88045a59a238 88045a590054 88045a59 88045a59
Call Trace:
 [] dump_stack+0x4d/0x72
 [] __warn+0xc6/0xe0
 [] warn_slowpath_fmt+0x4a/0x50
 [] ? hsw_audio_codec_disable+0xdd/0x110 [i915]
 [] intel_display_power_put+0xe8/0x100 [i915]
 [] intel_disable_ddi+0x46/0x80 [i915]
 [] haswell_crtc_disable+0x16f/0x290 [i915]
 [] intel_atomic_commit_tail+0x153/0x10e0 [i915]
 [] ? drm_atomic_helper_swap_state+0x140/0x2d0
 [] intel_atomic_commit+0x3fd/0x520 [i915]
 [] ? drm_atomic_add_affected_connectors+0x22/0xf0
 [] drm_atomic_commit+0x32/0x50
 [] restore_fbdev_mode+0x147/0x260
 [] drm_fb_helper_restore_fbdev_mode_unlocked+0x2e/0x70
 [] drm_fb_helper_set_par+0x28/0x50
 [] drm_fb_helper_hotplug_event+0x143/0x180
 [] intel_fbdev_output_poll_changed+0x15/0x20 [i915]
 [] drm_kms_helper_hotplug_event+0x22/0x30
 [] output_poll_execute+0x192/0x1e0
 [] process_one_work+0x14c/0x480
 [] worker_thread+0x24a/0x4e0
 [] ? process_one_work+0x480/0x480
 [] ? process_one_work+0x480/0x480
 [] kthread+0xc4/0xe0
 [] ret_from_fork+0x1f/0x40
 [] ? kthread_worker_fn+0x180/0x180
---[ end trace 2d440da5f0c053e4 ]---

Instead of scratching heads too much while CI is down, let's revert
before more trouble is caused.

Cc: Maarten Lankhorst 
Cc: Mika Kuoppala 
Cc: Ville Syrjälä 
Reported-by: Mika Kuoppala 
Reported-by: Ville Syrjälä 
Acked-by: Mika Kuoppala 
Acked-by: Ville Syrjälä 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_atomic.c| 66 -
 drivers/gpu/drm/drm_crtc.c  |  6 
 drivers/gpu/drm/drm_crtc_internal.h |  1 -
 3 files changed, 73 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 34cdbe154540..6712278c73f0 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1590,72 +1590,6 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
 }
 EXPORT_SYMBOL(drm_atomic_clean_old_fb);
 
-int drm_atomic_remove_fb(struct drm_framebuffer *fb)
-{
-   struct drm_modeset_acquire_ctx ctx;
-   struct drm_device *dev = fb->dev;
-   struct drm_atomic_state *state;
-   struct drm_plane *plane;
-   int ret = 0;
-   unsigned plane_mask;
-
-   state = drm_atomic_state_alloc(dev);
-   if (!state)
-   return -ENOMEM;
-
-   drm_modeset_acquire_init(, 0);
-   state->acquire_ctx = 
-
-retry:
-   plane_mask = 0;
-   ret = drm_modeset_lock_all_ctx(dev, );
-   if (ret)
-   goto unlock;
-
-   drm_for_each_plane(plane, dev) {
-   struct drm_plane_state *plane_state;
-
-   if (plane->state->fb != fb)
-   continue;
-
-   plane_state = drm_atomic_get_plane_state(state, plane);
-   if (IS_ERR(plane_state)) {
-   ret = PTR_ERR(plane_state);
-   goto unlock;
-   }
-
-   drm_atomic_set_fb_for_plane(plane_state, NULL);
-   ret = drm_atomic_set_crtc_for_plane(plane_state, NULL);
-   if (ret)
-   goto unlock;
-
-   plane_mask |= BIT(drm_plane_index(plane));
-
-   plane->old_fb = plane->fb;
-   plane->fb = NULL;
-   }
-
-   if (plane_mask)
-   ret = drm_atomic_commit(state);
-
-unlock:
-   if (plane_mask)
-   drm_atomic_clean_old_fb(dev, plane_mask, ret);
-
-   if (ret == -EDEADLK) {
-   drm_modeset_backoff();
-   goto retry;
-   }
-
-   if (ret || !plane_mask)
-   drm_atomic_state_free(state);
-
-   drm_modeset_drop_locks();
-   drm_modeset_acquire_fini();
-
-   return ret;
-}
-
 int drm_mode_atomic_ioctl(struct drm_device *dev,

Re: [Intel-gfx] ✗ Ro.CI.BAT: failure for drm/i915: remove superfluous i915_gem_object_free_mmap_offset call

[Joonas Lahtinen]

Merged, thanks for the patch and review.

Regards, Joonas

On to, 2016-07-14 at 13:14 +0100, Matthew Auld wrote:
> On 5 July 2016 at 14:21, Patchwork  wrote:
> > 
> > == Series Details ==
> > 
> > Series: drm/i915: remove superfluous i915_gem_object_free_mmap_offset call
> > URL   : https://patchwork.freedesktop.org/series/9516/
> > State : failure
> > 
> > == Summary ==
> > 
> > Series 9516v1 drm/i915: remove superfluous i915_gem_object_free_mmap_offset 
> > call
> > http://patchwork.freedesktop.org/api/1.0/series/9516/revisions/1/mbox
> > 
> > Test gem_exec_flush:
> > Subgroup basic-batch-kernel-default-cmd:
> > pass   -> FAIL   (ro-byt-n2820)
> https://bugs.freedesktop.org/show_bug.cgi?id=95372
> 
> > 
> > Subgroup basic-batch-kernel-default-wb:
> > pass   -> DMESG-FAIL (ro-bdw-i7-5557U)
> Created bug report https://bugs.freedesktop.org/show_bug.cgi?id=96927
> 
> > 
> > Test kms_pipe_crc_basic:
> > Subgroup suspend-read-crc-pipe-a:
> > skip   -> DMESG-WARN (ro-bdw-i7-5557U)
> Created bug report https://bugs.freedesktop.org/show_bug.cgi?id=96913
-- 
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 2/2] drm/i915: refactor eb_get_batch()

[Dave Gordon]

On 13/07/16 13:44, Chris Wilson wrote:

On Wed, Jul 13, 2016 at 02:38:16PM +0200, Daniel Vetter wrote:

On Thu, Jun 30, 2016 at 04:12:49PM +0100, Dave Gordon wrote:

Precursor for fix to secure batch execution. We will need to be able to
retrieve the batch VMA (as well as the batch itself) from the eb list,
so this patch extracts that part of eb_get_batch() into a separate
function, and moves both parts to a more logical place in the file, near
where the eb list is created.

Also, it may not be obvious, but the current execbuffer2 ioctl interface
requires that the buffer object containing the batch-to-be-executed be
the LAST entry in the exec2_list[] array (I expected it to be the first!).

To clarify this, we can replace the rather obscure construct
"list_entry(eb->vmas.prev, ...)"
in the old version of eb_get_batch() with the equivalent but more explicit
"list_last_entry(>vmas,...)"
in the new eb_get_batch_vma() and of course add an explanatory comment.

Signed-off-by: Dave Gordon 


I have no context on the secure batch fix you're talking about, but this
here makes sense as an independent cleanup.


It won't help though, so this is just churn for no purpose.
-Chris


At the very least, it replaces a confusing construct with
a comprehensible one annotated with an explanatory comment.

Separating finding the VMA for the batch from finding the batch itself
also improves clarity and costs nothing (compiler inlines it anyway).

Comprehensibility -- and hence maintainability -- is always
a worthwhile purpose :)

BTW, do the comments in this code from patch

d23db88 drm/i915: Prevent negative relocation deltas from wrapping

still apply? 'Cos I think it's pretty ugly to be setting a flag
on a VMA as a side-effect of a "lookup" type operation :( Surely
cleaner to do that sort of think at the top level i.e. inside
i915_gem_do_execbuffer() ?

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

Re: [Intel-gfx] [PATCH] drm/i915: Ignore panel type from OpRegion on SKL

[Ville Syrjälä]

On Tue, Jul 12, 2016 at 03:00:37PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Dell XPS 13 9350 apparently doesn't like it when we use the panel type
> from OpRegion. The OpRegion panel type (0) tells us to use use low
> vswing for eDP, whereas the VBT panel type (2) tells us to use normal
> vswing. The problem is that low vswing results in some display flickers.
> Since no one seems to know how this stuff is supposed to be handled,
> let's just ignore the OpRegion panel type on SKL for now.
> 
> Reported-by: James Bottomley 
> Cc: James Bottomley 
> Cc: drm-intel-fi...@lists.freedesktop.org
> References: 
> https://lists.freedesktop.org/archives/intel-gfx/2016-June/098826.html
> Fixes: a05628195a0d ("drm/i915: Get panel_type from OpRegion panel details")
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_opregion.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
> b/drivers/gpu/drm/i915/intel_opregion.c
> index c27d5eb063d0..259760f12d16 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -1072,5 +1072,16 @@ intel_opregion_get_panel_type(struct drm_i915_private 
> *dev_priv)
>   return -ENODEV;
>   }
>  
> + /*
> +  * FIXME On Dell XPS 13 9350 the OpRegion panel type (0) gives us
> +  * low vswing for eDP, whereas the VBT panel type (2) gives us normal
> +  * vswing instead. Low vswing results in some display flickers, so
> +  * let's simply ignore the OpRegion panel type on SKL for now.
> +  */
> + if (IS_SKYLAKE(dev_priv)) {
> + DRM_DEBUG_KMS("Ignoring OpRegion panel type (%d)\n", ret);

I did s/ret/ret - 1/ here to make the debug output correct, and pushed
the patch to dinq. Thanks for the review.

> + return -ENODEV;
> + }
> +
>   return ret - 1;
>  }
> -- 
> 2.7.4

-- 
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] ✗ Ro.CI.BAT: warning for drm/i915: Ignore panel type from OpRegion on SKL

[Ville Syrjälä]

On Tue, Jul 12, 2016 at 01:12:20PM -, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Ignore panel type from OpRegion on SKL
> URL   : https://patchwork.freedesktop.org/series/9752/
> State : warning
> 
> == Summary ==
> 
> Series 9752v1 drm/i915: Ignore panel type from OpRegion on SKL
> http://patchwork.freedesktop.org/api/1.0/series/9752/revisions/1/mbox
> 
> Test kms_flip:
> Subgroup basic-flip-vs-wf_vblank:
> fail   -> PASS   (ro-byt-n2820)
> Test kms_pipe_crc_basic:
> Subgroup suspend-read-crc-pipe-a:
> skip   -> DMESG-WARN (ro-bdw-i7-5557U)

This test regularly ping-pongs between warn and skip on this machine.

[  429.961534] [drm:intel_dp_link_training_clock_recovery [i915]] *ERROR* 
failed to enable link training
[  430.045093] [drm:intel_dp_start_link_train [i915]] *ERROR* failed to start 
channel equalization

> 
> fi-kbl-qkkr  total:237  pass:174  dwarn:28  dfail:1   fail:7   skip:27 
> fi-skl-i5-6260u  total:237  pass:218  dwarn:0   dfail:0   fail:7   skip:12 
> fi-skl-i7-6700k  total:237  pass:204  dwarn:0   dfail:0   fail:7   skip:26 
> fi-snb-i7-2600   total:237  pass:190  dwarn:0   dfail:0   fail:7   skip:40 
> ro-bdw-i5-5250u  total:237  pass:213  dwarn:1   dfail:0   fail:7   skip:16 
> ro-bdw-i7-5557U  total:237  pass:213  dwarn:2   dfail:0   fail:7   skip:15 
> ro-bdw-i7-5600u  total:237  pass:199  dwarn:0   dfail:0   fail:7   skip:31 
> ro-bsw-n3050 total:218  pass:170  dwarn:0   dfail:0   fail:5   skip:42 
> ro-byt-n2820 total:237  pass:189  dwarn:0   dfail:0   fail:10  skip:38 
> ro-hsw-i3-4010u  total:237  pass:206  dwarn:0   dfail:0   fail:7   skip:24 
> ro-hsw-i7-4770r  total:237  pass:206  dwarn:0   dfail:0   fail:7   skip:24 
> ro-ilk-i7-620lm  total:237  pass:166  dwarn:0   dfail:0   fail:8   skip:63 
> ro-ilk1-i5-650   total:232  pass:166  dwarn:0   dfail:0   fail:8   skip:58 
> ro-ivb-i7-3770   total:237  pass:197  dwarn:0   dfail:0   fail:7   skip:33 
> ro-skl3-i5-6260u total:237  pass:217  dwarn:1   dfail:0   fail:7   skip:12 
> ro-snb-i7-2620M  total:237  pass:188  dwarn:0   dfail:0   fail:8   skip:41 
> 
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1473/
> 
> 7d4e6e0 drm-intel-nightly: 2016y-07m-12d-11h-23m-08s UTC integration manifest
> 5bd5ff5 drm/i915: Ignore panel type from OpRegion on SKL

-- 
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 v3] drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl)

[Daniel Vetter]

On Thu, Jul 14, 2016 at 11:11:02AM +0100, Chris Wilson wrote:
> On Thu, Jul 14, 2016 at 10:59:04AM +0100, Chris Wilson wrote:
> > On Thu, Jul 14, 2016 at 10:12:17AM +0200, Daniel Vetter wrote:
> > > On Thu, Jul 14, 2016 at 08:04:19AM +0100, Chris Wilson wrote:
> > > > vGEM buffers are useful for passing data between software clients and
> > > > hardware renders. By allowing the user to create and attach fences to
> > > > the exported vGEM buffers (on the dma-buf), the user can implement a
> > > > deferred renderer and queue hardware operations like flipping and then
> > > > signal the buffer readiness (i.e. this allows the user to schedule
> > > > operations out-of-order, but have them complete in-order).
> > > > 
> > > > This also makes it much easier to write tightly controlled testcases for
> > > > dma-buf fencing and signaling between hardware drivers.
> > > > 
> > > > v2: Don't pretend the fences exist in an ordered timeline, but allocate
> > > > a separate fence-context for each fence so that the fences are
> > > > unordered.
> > > > v3: Make the debug output more interesting, and so the signaled status.
> > > > 
> > > > Testcase: igt/vgem_basic/dmabuf-fence
> > > > Signed-off-by: Chris Wilson 
> > > > Cc: Sean Paul 
> > > > Cc: Zach Reizner 
> > > > Cc: Gustavo Padovan 
> > > > Cc: Daniel Vetter 
> > > > Acked-by: Zach Reizner 
> > > 
> > > One thing I completely forgotten: This allows userspace to hang kernel
> > > drivers. i915 (and other gpu drivers) can recover using hangcheck, but
> > > dumber drivers (v4l, if that ever happens) probably never except such a
> > > case. We've had a similar discusion with the userspace fences exposed in
> > > sw_fence, and decided to move all those ioctl into debugfs. I think we
> > > should do the same for this vgem-based debugging of implicit sync. Sorry
> > > for realizing this this late.
> > 
> > One of the very tests I make is to ensure that we recover from such a
> > hang. I don't see the difference between this any of the other ways
> > userspace can shoot itself (and others) in the foot.
> 
> So one solution would be to make vgem fences automatically timeout (with
> a flag for root to override for the sake of testing hang detection).

The problem is other drivers. E.g. right now atomic helpers assume that
fences will signal, and can't recover if they don't. This is why drivers
where things might fail must have some recovery (hangcheck, timeout) to
make sure dma_fences always signal.

Imo not even root should be allowed to break this, since it could put
drivers into a non-recoverable state. I think this must be restricted to
something known-unsafe-don't-enable-on-production like debugfs.

Other solutions which I don't like:
- Everyone needs to be able to recover. Given how much effort it is to
  just keep i915 hangcheck in working order I think that's totally
  illusionary to assume. At least once world+dog (atomic, v4l, ...) all
  consume/produce fences, subsystems where the usual assumption holds that
  async ops complete.

- Really long timeouts are allowed for root in vgem. Could lead to even
  more fun in testing i915 hangchecks I think, so don't like that much
  either.

I think the best option is to just do the same as we've done for sw_fence,
and move it to debugfs. We could reuse the debugfs sw_fence interface to
create them (gives us more control as a bonus), and just have an ioctl to
attach fences to vgem (which could be unpriviledged).
-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

[Intel-gfx] ✗ Ro.CI.BAT: failure for drm/i915: Move hangcheck reset from out of the engines into the GPU reset

[Patchwork]

== Series Details ==

Series: drm/i915: Move hangcheck reset from out of the engines into the GPU 
reset
URL   : https://patchwork.freedesktop.org/series/9871/
State : failure

== Summary ==

Series 9871v1 drm/i915: Move hangcheck reset from out of the engines into the 
GPU reset
http://patchwork.freedesktop.org/api/1.0/series/9871/revisions/1/mbox

Test drv_getparams_basic:
Subgroup basic-eu-total:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-subslice-total:
pass   -> SKIP   (fi-skl-i5-6260u)
Test drv_hangman:
Subgroup error-state-basic:
pass   -> SKIP   (fi-skl-i5-6260u)
Test drv_module_reload_basic:
dmesg-warn -> TIMEOUT(fi-skl-i5-6260u)
Test gem_basic:
Subgroup bad-close:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup create-close:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup create-fd-close:
pass   -> SKIP   (fi-skl-i5-6260u)
Test gem_busy:
Subgroup basic-blt:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-bsd:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-bsd1:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-bsd2:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-parallel-blt:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-parallel-bsd:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-parallel-bsd1:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-parallel-bsd2:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-parallel-render:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-parallel-vebox:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-render:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-vebox:
pass   -> SKIP   (fi-skl-i5-6260u)
Test gem_close_race:
Subgroup basic-process:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-threads:
pass   -> SKIP   (fi-skl-i5-6260u)
Test gem_cs_tlb:
Subgroup basic-default:
pass   -> SKIP   (fi-skl-i5-6260u)
Test gem_ctx_exec:
Subgroup basic:
pass   -> SKIP   (fi-skl-i5-6260u)
Test gem_ctx_param:
Subgroup basic:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-default:
pass   -> SKIP   (fi-skl-i5-6260u)
Test gem_exec_basic:
Subgroup basic-blt:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-bsd:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-bsd1:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-bsd2:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-default:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-render:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup basic-vebox:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup gtt-blt:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup gtt-bsd:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup gtt-bsd1:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup gtt-bsd2:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup gtt-default:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup gtt-render:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup gtt-vebox:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup readonly-blt:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup readonly-bsd:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup readonly-bsd1:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup readonly-bsd2:
pass   -> SKIP   (fi-skl-i5-6260u)
Subgroup readonly-default:
pass   -> SKIP   (fi-skl-i5-6260u)
WARNING: Long output truncated

Results at /archive/results/CI_IGT_test/RO_Patchwork_1491/

c504630 drm-intel-nightly: 2016y-07m-14d-10h-18m-17s UTC integration manifest
9f5f1b2 drm/i915: Move hangcheck reset from out of the engines into the GPU 
reset

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

[Intel-gfx] drm-intel.git Committers: Reminder about tagging bugfixes

[Daniel Vetter]

Hi all,

Apparently this wasn't known to everyone, hence this small reminder
about correctly tagging bugfixes when pushing them:
- add a Bugzilla: or References: link for any bug reported anywhere
(bugzilla or mailing list). Also add Reported-by:/Tested-by: for
credits.
- add Fixes: if it's a regression fix. Run "dim fixes $sha1" for
convenience, to generate this correctly
- add correct Cc: tags, Cc: stable for backporting to all kernels
(including -rc), Cc: drm-intel-fixes if it's only needed in current
-rc series.

Special note about pre-production hw: We only backport fixes when the
prelim_hw_support tag is lifted, before that point we don't bother.
But after that _all_ bugfixes need one of the above tags, and right
now both kbl and apl/bxt aren't preliminary any more!

For full details see the docs:
https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-intel.html
If you haven't read them in a while would be good to check them out
again, quite a few improvements and clarifications over the past few
months.

Thanks, Daniel

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

Re: [Intel-gfx] [PATCH v3] drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl)

[Chris Wilson]

On Thu, Jul 14, 2016 at 11:11:02AM +0100, Chris Wilson wrote:
> So one solution would be to make vgem fences automatically timeout (with
> a flag for root to override for the sake of testing hang detection).

diff --git a/drivers/gpu/drm/vgem/vgem_fence.c 
b/drivers/gpu/drm/vgem/vgem_fence.c
index b7da11419ad6..17c63c9a8ea0 100644
--- a/drivers/gpu/drm/vgem/vgem_fence.c
+++ b/drivers/gpu/drm/vgem/vgem_fence.c
@@ -28,6 +28,7 @@
 struct vgem_fence {
struct fence base;
struct spinlock lock;
+   struct timer_list timer;
 };
 
 static const char *vgem_fence_get_driver_name(struct fence *fence)
@@ -50,6 +51,14 @@ static bool vgem_fence_enable_signaling(struct fence *fence)
return true;
 }
 
+static void vgem_fence_release(struct fence *base)
+{
+   struct vgem_fence *fence = container_of(base, typeof(*fence), base);
+
+   del_timer_sync(>timer);
+   fence_free(>base);
+}
+
 static void vgem_fence_value_str(struct fence *fence, char *str, int size)
 {
snprintf(str, size, "%u", fence->seqno);
@@ -67,11 +76,21 @@ const struct fence_ops vgem_fence_ops = {
.enable_signaling = vgem_fence_enable_signaling,
.signaled = vgem_fence_signaled,
.wait = fence_default_wait,
+   .release = vgem_fence_release,
+
.fence_value_str = vgem_fence_value_str,
.timeline_value_str = vgem_fence_timeline_value_str,
 };
 
-static struct fence *vgem_fence_create(struct vgem_file *vfile)
+static void vgem_fence_timeout(unsigned long data)
+{
+   struct vgem_fence *fence = (struct vgem_fence *)data;
+
+   fence_signal(>base);
+}
+
+static struct fence *vgem_fence_create(struct vgem_file *vfile,
+  unsigned int flags)
 {
struct vgem_fence *fence;
 
@@ -83,6 +102,12 @@ static struct fence *vgem_fence_create(struct vgem_file 
*vfile)
fence_init(>base, _fence_ops, >lock,
   fence_context_alloc(1), 1);
 
+   setup_timer(>timer, vgem_fence_timeout, (unsigned long)fence);
+
+   /* We force the fence to expire within 10s to prevent driver hangs */
+   if (!(flags & VGEM_FENCE_NOTIMEOUT))
+   mod_timer(>timer, 10*HZ);
+
return >base;
 }
 
@@ -114,9 +139,12 @@ int vgem_fence_attach_ioctl(struct drm_device *dev,
struct fence *fence;
int ret;
 
-   if (arg->flags & ~VGEM_FENCE_WRITE)
+   if (arg->flags & ~(VGEM_FENCE_WRITE | VGEM_FENCE_NOTIMEOUT))
return -EINVAL;
 
+   if (arg->flags & VGEM_FENCE_NOTIMEOUT && !capable(CAP_SYS_ADMIN))
+   return -EPERM;
+
if (arg->pad)
return -EINVAL;
 
@@ -128,7 +156,7 @@ int vgem_fence_attach_ioctl(struct drm_device *dev,
if (ret)
goto out;
 
-   fence = vgem_fence_create(vfile);
+   fence = vgem_fence_create(vfile, arg->flags);
if (!fence) {
ret = -ENOMEM;
goto out;
diff --git a/include/uapi/drm/vgem_drm.h b/include/uapi/drm/vgem_drm.h
index 352d2fae8de9..55fd08750773 100644
--- a/include/uapi/drm/vgem_drm.h
+++ b/include/uapi/drm/vgem_drm.h
@@ -45,7 +45,8 @@ extern "C" {
 struct drm_vgem_fence_attach {
__u32 handle;
__u32 flags;
-#define VGEM_FENCE_WRITE 0x1
+#define VGEM_FENCE_WRITE   0x1
+#define VGEM_FENCE_NOTIMEOUT   0x2
__u32 out_fence;
__u32 pad;
 };


-- 
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] ✗ Ro.CI.BAT: failure for drm/i915: remove superfluous i915_gem_object_free_mmap_offset call

[Matthew Auld]

On 5 July 2016 at 14:21, Patchwork  wrote:
> == Series Details ==
>
> Series: drm/i915: remove superfluous i915_gem_object_free_mmap_offset call
> URL   : https://patchwork.freedesktop.org/series/9516/
> State : failure
>
> == Summary ==
>
> Series 9516v1 drm/i915: remove superfluous i915_gem_object_free_mmap_offset 
> call
> http://patchwork.freedesktop.org/api/1.0/series/9516/revisions/1/mbox
>
> Test gem_exec_flush:
> Subgroup basic-batch-kernel-default-cmd:
> pass   -> FAIL   (ro-byt-n2820)
https://bugs.freedesktop.org/show_bug.cgi?id=95372

> Subgroup basic-batch-kernel-default-wb:
> pass   -> DMESG-FAIL (ro-bdw-i7-5557U)
Created bug report https://bugs.freedesktop.org/show_bug.cgi?id=96927

> Test kms_pipe_crc_basic:
> Subgroup suspend-read-crc-pipe-a:
> skip   -> DMESG-WARN (ro-bdw-i7-5557U)
Created bug report https://bugs.freedesktop.org/show_bug.cgi?id=96913
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Move hangcheck reset from out of the engines into the GPU reset

[Chris Wilson]

Let's not reset the hangcheck as a side-effect of initialising the
engine, but as part of our GPU reset.

Signed-off-by: Chris Wilson 
Cc: Mika Kuoppala 
Cc: Arun Siluvery 
Cc: Tvrtko Ursulin 
---
 drivers/gpu/drm/i915/i915_gem.c | 1 +
 drivers/gpu/drm/i915/intel_lrc.c| 2 --
 drivers/gpu/drm/i915/intel_ringbuffer.c | 2 --
 3 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b788f97cd4cf..978fa6f6f747 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3169,6 +3169,7 @@ static void i915_gem_reset_engine_cleanup(struct 
intel_engine_cs *engine)
}
 
intel_ring_init_seqno(engine, engine->last_submitted_seqno);
+   intel_engine_init_hangcheck(engine);
 }
 
 void i915_gem_reset(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index b6af635e3a0f..377cfbfb71fd 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1529,8 +1529,6 @@ static int gen8_init_common_ring(struct intel_engine_cs 
*engine)
engine->next_context_status_buffer = next_context_status_buffer_hw;
DRM_DEBUG_DRIVER("Execlists enabled for %s\n", engine->name);
 
-   intel_engine_init_hangcheck(engine);
-
return intel_mocs_init_engine(engine);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 94c8ef461721..8a9e035341a9 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -627,8 +627,6 @@ static int init_ring_common(struct intel_engine_cs *engine)
ringbuf->tail = I915_READ_TAIL(engine) & TAIL_ADDR;
intel_ring_update_space(ringbuf);
 
-   intel_engine_init_hangcheck(engine);
-
 out:
intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 
-- 
2.8.1

___
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: unify first-stage engine struct setup

[Dave Gordon]

On 13/07/16 14:16, Tvrtko Ursulin wrote:


On 13/07/16 13:23, Daniel Vetter wrote:

On Fri, Jul 01, 2016 at 05:47:11PM +0100, Tvrtko Ursulin wrote:

From: Dave Gordon 


[snip]

  {
-const struct logical_ring_info *info = _rings[id];
+const struct engine_info *info = _engines[id];
  struct intel_engine_cs *engine = _priv->engine[id];
-enum forcewake_domains fw_domains;

  engine->id = id;
+engine->i915 = dev_priv;
  engine->name = info->name;
  engine->exec_id = info->exec_id;
-engine->guc_id = info->guc_id;
+engine->hw_id = engine->guc_id = info->guc_id;


Optional bikeshed: s/info->guc_id/info->hw_id/ makes sense imo in the new
context. Or nuking engine->guc_id.


Someone said we cannot be sure they will be the same in the future. So
maybe just rename to hw_id for now.

Regards,
Tvrtko


The GuC firmware *could* use a completely different enumeration of the 
engines, but why would it? Since it's closely tied to the hardware, it 
*ought* to use the same naming scheme as the h/w. So I have no objection 
to getting rid of guc_id entirely, and changing existing uses to use 
hw_id instead.


OTOH it seems little benefit and would certainly involve more work to 
reverse. The firmware team might, after all, decide that they too want 
to decouple the logical-engine-numbers used for the KMD interface from 
whatever the hardware team decide is the best way to number engines -- 
which might after all change between generations or even different SKUs 
of the same device!


SO, I think the "best" version of that line is probably:

+   engine->hw_id = info->hw_id;
+
+   /* Current GuC f/w uses hw_id not driver id */
+   engine->guc_id = info->hw_id;

and we'll add "info->guc_id" back again if it ever becomes necessary.

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

Re: [Intel-gfx] [PACTH i-g-t] lib/igt_kms: Fix different order of properties and their name strings

[Marius Vlad]

Applied. Thanks!
On Mon, Jul 11, 2016 at 03:54:12PM -0400, Robert Foss wrote:
> This is a reminder of this series.
> 
> On 2016-06-29 07:22 AM, robert.f...@collabora.com wrote:
> >From: Robert Foss 
> >
> >igt_crtc_prop_names and igt_atomic_crtc_properties have different orders of
> >properties, which is fixed in this patch.
> >
> >Signed-off-by: Robert Foss 
> >---
> >  lib/igt_kms.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> >index af72b90..dd4eb84 100644
> >--- a/lib/igt_kms.c
> >+++ b/lib/igt_kms.c
> >@@ -164,8 +164,8 @@ static const char 
> >*igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
> >
> >  static const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
> > "background_color",
> >-"DEGAMMA_LUT",
> > "CTM",
> >+"DEGAMMA_LUT",
> > "GAMMA_LUT",
> >  };
> >
> >


signature.asc
Description: Digital signature
___
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/fbdev: Drain the suspend worker on retiring

[Mika Kuoppala]

Chris Wilson  writes:

> Since the suspend_work can arm itself if the console_lock() is currently
> held elsewhere, simply calling flush_work() doesn't guarantee that the
> work is idle upon return. To do so requires using cancel_work_sync().
>
> Signed-off-by: Chris Wilson 
> Cc: Daniel Vetter 
> Cc: Mika Kuoppala 

Reviewed-by: Mika Kuoppala 

> ---
>  drivers/gpu/drm/i915/intel_fbdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c 
> b/drivers/gpu/drm/i915/intel_fbdev.c
> index 86b00c6db1a6..ef17d88a1bc7 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -768,7 +768,7 @@ void intel_fbdev_fini(struct drm_device *dev)
>   if (!ifbdev)
>   return;
>  
> - flush_work(_priv->fbdev_suspend_work);
> + cancel_work_sync(_priv->fbdev_suspend_work);
>   if (!current_is_async())
>   intel_fbdev_sync(ifbdev);
>  
> -- 
> 2.8.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH igt 01/17] lib: Start weaning off defunct intel_chipset.h

[Chris Wilson]

On Thu, Jul 14, 2016 at 11:31:08AM +0100, Emil Velikov wrote:
> On 13 July 2016 at 14:34, Chris Wilson  wrote:
> > On Wed, Jul 13, 2016 at 02:40:49PM +0200, Daniel Vetter wrote:
> >> On Wed, Jun 29, 2016 at 12:38:51PM +0100, Chris Wilson wrote:
> >> > Several years ago we made the plan of only having one canonical source
> >> > for i915_pciids.h, the kernel and everyone importing their definitions
> >> > from that. For consistency, we style the intel_device_info after the
> >> > kernel, most notably using a generation mask and a per-codename bitfield.
> >> >
> >> > This first step converts looking up the generation for a devid tree from
> >> > a massive if(devid)-chain to a (cached) table lookup.
> >> >
> >> > Signed-off-by: Chris Wilson 
> >>
> >> Should we extract the kernels list of chipset into intel_chipset.c too, to
> >> make them 1:1 copies like with i915_pciids.h? With that this (entire
> >> series) here has my ack. Without I'm not sold that much on the churn
> >> really.
> >
> > Hmm, next up would be libdrm which is almost a duplication of igt (and
> > can be massaged to be so). The next step would be feeding back the
> > commonality into the kernel, so that we really do have a single location
> > where we need to add new defines that can just percolate through.
> >
> > So I think you mean to have the device-info stanzas shared...
> 
> Fwiw I would suggest the same thing - have this this code in a single
> place (be that kernel or libdrm) accessible by everyone. Otherwise
> others (libva/beignet/mesa?) will copy it and it'll be a constant
> chance to keep it in sync.

That's what we said many years ago! Still trying to get there.
 
> About the patch itself here are a few small nitpicks:
>  - intel_i81x_info seems to be unused

One day! One day we will have that universal kms driver!

(It's required for the ddx at least, so I should hook up the ids as well
but really needs inclusion in i915_pciids.h first.)

>  - s/aaglelake/eaglelake/
>  - capitalise the first letter of each codename ?

Code already utilized codenames with the lowercase as filenames (for
register sets). I thought about making it call lowercase() - it was just
easier to match captilisation.

>  - Wikipedia lists Bearlake and Lakeport in both Gen3 and Gen4. Is it
> busted or ...

Or a confusion between chipset and GPU.

From bspec,

915G, Grantsdale-G, GDG
915GM, Alviso, ALV
945G, Lakport, LPT,
945GM, Callistoga, CST

Bearlake-B is listed for gen3, but that chipset shared more in common
with the early gen4 GMCH.

>  - Ironlake, Clarkdale is listed as Ironlake, while Ironlake,
> Arrandale as Arrandale. Guess those are the names people are more used
> to ?

It's just DevILK in the bspec, I'd forgotten Clarkdale but remembered
Arrandale as the mobile variant (and I wasn't actually sure if Clarkdale
was just the GPU-less chips). But since it is DevILK, we probably
should just use Ironlake, hmm or we list all the shortnames as well for
better cross-referencing.
-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

[Intel-gfx] [PATCH] Bump libdrm-intel dependency library to a newer version that support softpin.

[Marius Vlad]

Required by commit 2603b98ca (aubdump: Support softpin bos).

Signed-off-by: Marius Vlad 
CC: Kristian Høgsberg Kristensen 
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index f05bcb0..ade9756 100644
--- a/configure.ac
+++ b/configure.ac
@@ -119,7 +119,7 @@ if test "x$GCC" = "xyes"; then
 fi
 AC_SUBST(ASSEMBLER_WARN_CFLAGS)
 
-PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm])
+PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.68 libdrm])
 PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
 
 case "$target_cpu" in
-- 
2.8.1

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

Re: [Intel-gfx] [PATCH igt 01/17] lib: Start weaning off defunct intel_chipset.h

[Emil Velikov]

On 13 July 2016 at 14:34, Chris Wilson  wrote:
> On Wed, Jul 13, 2016 at 02:40:49PM +0200, Daniel Vetter wrote:
>> On Wed, Jun 29, 2016 at 12:38:51PM +0100, Chris Wilson wrote:
>> > Several years ago we made the plan of only having one canonical source
>> > for i915_pciids.h, the kernel and everyone importing their definitions
>> > from that. For consistency, we style the intel_device_info after the
>> > kernel, most notably using a generation mask and a per-codename bitfield.
>> >
>> > This first step converts looking up the generation for a devid tree from
>> > a massive if(devid)-chain to a (cached) table lookup.
>> >
>> > Signed-off-by: Chris Wilson 
>>
>> Should we extract the kernels list of chipset into intel_chipset.c too, to
>> make them 1:1 copies like with i915_pciids.h? With that this (entire
>> series) here has my ack. Without I'm not sold that much on the churn
>> really.
>
> Hmm, next up would be libdrm which is almost a duplication of igt (and
> can be massaged to be so). The next step would be feeding back the
> commonality into the kernel, so that we really do have a single location
> where we need to add new defines that can just percolate through.
>
> So I think you mean to have the device-info stanzas shared...

Fwiw I would suggest the same thing - have this this code in a single
place (be that kernel or libdrm) accessible by everyone. Otherwise
others (libva/beignet/mesa?) will copy it and it'll be a constant
chance to keep it in sync.

About the patch itself here are a few small nitpicks:
 - intel_i81x_info seems to be unused
 - s/aaglelake/eaglelake/
 - capitalise the first letter of each codename ?
 - Wikipedia lists Bearlake and Lakeport in both Gen3 and Gen4. Is it
busted or ...
 - Ironlake, Clarkdale is listed as Ironlake, while Ironlake,
Arrandale as Arrandale. Guess those are the names people are more used
to ?

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

Re: [Intel-gfx] [PATCH 5/8] drm/i915: Define a separate variable and control for RPS waitboost frequency

[Mika Kuoppala]

Chris Wilson  writes:

> To allow the user finer control over waitboosting, allow them to set the
> frequency we request for the boost. This also them allows to effectively
> disable the boosting by setting the boost request to a low frequency.
>
> Signed-off-by: Chris Wilson 

Reviewed-by: Mika Kuoppala 

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |  2 ++
>  drivers/gpu/drm/i915/i915_drv.h |  1 +
>  drivers/gpu/drm/i915/i915_irq.c |  9 +
>  drivers/gpu/drm/i915/i915_sysfs.c   | 38 
> +
>  drivers/gpu/drm/i915/intel_pm.c |  5 -
>  5 files changed, 50 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 844fea795bae..d1ff4cb9b90e 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1381,6 +1381,8 @@ static int i915_frequency_info(struct seq_file *m, void 
> *unused)
>  intel_gpu_freq(dev_priv, dev_priv->rps.idle_freq));
>   seq_printf(m, "Min freq: %d MHz\n",
>  intel_gpu_freq(dev_priv, dev_priv->rps.min_freq));
> + seq_printf(m, "Boost freq: %d MHz\n",
> +intel_gpu_freq(dev_priv, dev_priv->rps.boost_freq));
>   seq_printf(m, "Max freq: %d MHz\n",
>  intel_gpu_freq(dev_priv, dev_priv->rps.max_freq));
>   seq_printf(m,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e76cfe2e2471..fe85235367be 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1170,6 +1170,7 @@ struct intel_gen6_power_mgmt {
>   u8 max_freq_softlimit;  /* Max frequency permitted by the driver */
>   u8 max_freq;/* Maximum frequency, RP0 if not overclocking */
>   u8 min_freq;/* AKA RPn. Minimum frequency */
> + u8 boost_freq;  /* Frequency to request when wait boosting */
>   u8 idle_freq;   /* Frequency to request when we are idle */
>   u8 efficient_freq;  /* AKA RPe. Pre-determined balanced frequency */
>   u8 rp1_freq;/* "less than" RP0 power/freqency */
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 1c2aec392412..c8ed36765301 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1105,9 +1105,10 @@ static void gen6_pm_rps_work(struct work_struct *work)
>   new_delay = dev_priv->rps.cur_freq;
>   min = dev_priv->rps.min_freq_softlimit;
>   max = dev_priv->rps.max_freq_softlimit;
> -
> - if (client_boost) {
> - new_delay = dev_priv->rps.max_freq_softlimit;
> + if (client_boost || any_waiters(dev_priv))
> + max = dev_priv->rps.max_freq;
> + if (client_boost && new_delay < dev_priv->rps.boost_freq) {
> + new_delay = dev_priv->rps.boost_freq;
>   adj = 0;
>   } else if (pm_iir & GEN6_PM_RP_UP_THRESHOLD) {
>   if (adj > 0)
> @@ -1122,7 +1123,7 @@ static void gen6_pm_rps_work(struct work_struct *work)
>   new_delay = dev_priv->rps.efficient_freq;
>   adj = 0;
>   }
> - } else if (any_waiters(dev_priv)) {
> + } else if (client_boost || any_waiters(dev_priv)) {
>   adj = 0;
>   } else if (pm_iir & GEN6_PM_RP_DOWN_TIMEOUT) {
>   if (dev_priv->rps.cur_freq > dev_priv->rps.efficient_freq)
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c 
> b/drivers/gpu/drm/i915/i915_sysfs.c
> index d61829e54f93..8c045ff47f0e 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -318,6 +318,41 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
>   return snprintf(buf, PAGE_SIZE, "%d\n", ret);
>  }
>  
> +static ssize_t gt_boost_freq_mhz_show(struct device *kdev, struct 
> device_attribute *attr, char *buf)
> +{
> + struct drm_minor *minor = dev_to_drm_minor(kdev);
> + struct drm_i915_private *dev_priv = to_i915(minor->dev);
> +
> + return snprintf(buf, PAGE_SIZE, "%d\n",
> + intel_gpu_freq(dev_priv, dev_priv->rps.boost_freq));
> +}
> +
> +static ssize_t gt_boost_freq_mhz_store(struct device *kdev,
> +struct device_attribute *attr,
> +const char *buf, size_t count)
> +{
> + struct drm_minor *minor = dev_to_drm_minor(kdev);
> + struct drm_device *dev = minor->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + u32 val;
> + ssize_t ret;
> +
> + ret = kstrtou32(buf, 0, );
> + if (ret)
> + return ret;
> +
> + /* Validate against (static) hardware limits */
> + val = intel_freq_opcode(dev_priv, val);
> + if (val < 

Re: [Intel-gfx] ✓ Ro.CI.BAT: success for series starting with [1/7] drm/i915: unify first-stage engine struct setup

[Tvrtko Ursulin]

On 13/07/16 16:57, Patchwork wrote:

== Series Details ==

Series: series starting with [1/7] drm/i915: unify first-stage engine struct 
setup
URL   : https://patchwork.freedesktop.org/series/9821/
State : success

== Summary ==

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

Test prime_self_import:
 Subgroup basic-llseek-size:
 incomplete -> PASS   (ro-byt-n2820)

fi-kbl-qkkr  total:237  pass:174  dwarn:25  dfail:4   fail:7   skip:27
fi-skl-i7-6700k  total:237  pass:200  dwarn:2   dfail:0   fail:9   skip:26
fi-snb-i7-2600   total:237  pass:188  dwarn:0   dfail:0   fail:9   skip:40
ro-bdw-i5-5250u  total:237  pass:210  dwarn:2   dfail:0   fail:9   skip:16
ro-bdw-i7-5557U  total:237  pass:211  dwarn:1   dfail:0   fail:9   skip:16
ro-bdw-i7-5600u  total:237  pass:195  dwarn:2   dfail:0   fail:9   skip:31
ro-bsw-n3050 total:217  pass:170  dwarn:0   dfail:0   fail:4   skip:42
ro-byt-n2820 total:237  pass:178  dwarn:0   dfail:0   fail:7   skip:38
ro-hsw-i3-4010u  total:237  pass:199  dwarn:5   dfail:2   fail:7   skip:24
ro-hsw-i7-4770r  total:237  pass:204  dwarn:0   dfail:0   fail:9   skip:24
ro-ilk-i7-620lm  total:237  pass:166  dwarn:0   dfail:0   fail:8   skip:63
ro-ilk1-i5-650   total:232  pass:166  dwarn:0   dfail:0   fail:8   skip:58
ro-ivb-i7-3770   total:237  pass:197  dwarn:0   dfail:0   fail:7   skip:33
ro-skl3-i5-6260u total:237  pass:213  dwarn:3   dfail:0   fail:9   skip:12
ro-snb-i7-2620M  total:237  pass:187  dwarn:0   dfail:0   fail:9   skip:41

Results at /archive/results/CI_IGT_test/RO_Patchwork_1486/

5cd2699 drm-intel-nightly: 2016y-07m-13d-14h-43m-27s UTC integration manifest
f0b1666 drm/i915: Pull out some more common engine init code
be8c237 drm/i915: Move common engine setup into intel_engine_cs.c
a4cb7bc drm/i915: Simplify intel_init_ring_buffer prototype
d3dfae1 drm/i915: Make more use of the shared engine irq setup
ef59023 drm/i915: Unify engine init loop
ac58b8a drm/i915: Prepare for engine init unification
8120dcf drm/i915: unify first-stage engine struct setup


Merged to dinq, thanks for the patches and review!

Regards,

Tvrtko

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

Re: [Intel-gfx] [PATCH v3] drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl)

[Chris Wilson]

On Thu, Jul 14, 2016 at 10:59:04AM +0100, Chris Wilson wrote:
> On Thu, Jul 14, 2016 at 10:12:17AM +0200, Daniel Vetter wrote:
> > On Thu, Jul 14, 2016 at 08:04:19AM +0100, Chris Wilson wrote:
> > > vGEM buffers are useful for passing data between software clients and
> > > hardware renders. By allowing the user to create and attach fences to
> > > the exported vGEM buffers (on the dma-buf), the user can implement a
> > > deferred renderer and queue hardware operations like flipping and then
> > > signal the buffer readiness (i.e. this allows the user to schedule
> > > operations out-of-order, but have them complete in-order).
> > > 
> > > This also makes it much easier to write tightly controlled testcases for
> > > dma-buf fencing and signaling between hardware drivers.
> > > 
> > > v2: Don't pretend the fences exist in an ordered timeline, but allocate
> > > a separate fence-context for each fence so that the fences are
> > > unordered.
> > > v3: Make the debug output more interesting, and so the signaled status.
> > > 
> > > Testcase: igt/vgem_basic/dmabuf-fence
> > > Signed-off-by: Chris Wilson 
> > > Cc: Sean Paul 
> > > Cc: Zach Reizner 
> > > Cc: Gustavo Padovan 
> > > Cc: Daniel Vetter 
> > > Acked-by: Zach Reizner 
> > 
> > One thing I completely forgotten: This allows userspace to hang kernel
> > drivers. i915 (and other gpu drivers) can recover using hangcheck, but
> > dumber drivers (v4l, if that ever happens) probably never except such a
> > case. We've had a similar discusion with the userspace fences exposed in
> > sw_fence, and decided to move all those ioctl into debugfs. I think we
> > should do the same for this vgem-based debugging of implicit sync. Sorry
> > for realizing this this late.
> 
> One of the very tests I make is to ensure that we recover from such a
> hang. I don't see the difference between this any of the other ways
> userspace can shoot itself (and others) in the foot.

So one solution would be to make vgem fences automatically timeout (with
a flag for root to override for the sake of testing hang detection).
-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 6/7] drm/i915: Move common engine setup into intel_engine_cs.c

[Chris Wilson]

On Thu, Jul 14, 2016 at 10:25:39AM +0100, Tvrtko Ursulin wrote:
> 
> On 13/07/16 17:04, Chris Wilson wrote:
> >On Wed, Jul 13, 2016 at 04:03:40PM +0100, Tvrtko Ursulin wrote:
> >>+   /*
> >>+* Catch failures to update intel_engines table when the new engines
> >>+* are added to the driver by a warning and disabling the forgotten
> >>+* engines.
> >>+*/
> >>+   if (WARN_ON(mask != INTEL_INFO(dev_priv)->ring_mask)) {
> >>+   struct intel_device_info *info =
> >>+   (struct intel_device_info *)_priv->info;
> >
> >I snuck in mkwrite_device_info(), so
> >
> >if (WARN_ON(mask != INTEL_INFO(dev_priv)->ring_mask))
> > mkwrite_device_info(dev_priv)->ring_mask = mask;
> 
> This part is just code movement, the block you quote exists before
> this series even!
> 
> Follow up patch to this series would be easiest then, or a solitary
> precursor if you insist. Dangers of code movement with edits huh?
> (94b4f3ba483ace6dd4a3f881e19cc18bdbafa6ef)

I was also on a checkpatch witchhunt.
-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 1/7] drm/i915: unify first-stage engine struct setup

[Chris Wilson]

On Thu, Jul 14, 2016 at 10:19:16AM +0100, Tvrtko Ursulin wrote:
> 
> On 13/07/16 17:01, Chris Wilson wrote:
> >On Wed, Jul 13, 2016 at 04:03:35PM +0100, Tvrtko Ursulin wrote:
> >>From: Dave Gordon 
> >>
> >>intel_lrc.c has a table of "logical rings" (meaning engines), while
> >>intel_ringbuffer.c has separately open-coded initialisation for each
> >>engine. We can deduplicate this somewhat by using the same first-stage
> >>engine-setup function for both modes.
> >>
> >>So here we expose the function that transfers information from the
> >>static table of (all) known engines to the dev_priv->engine array of
> >>engines available on this device (adjusting the names along the way)
> >>and then embed calls to it in both the LRC and the legacy-mode setup.
> >>
> >>Signed-off-by: Dave Gordon 
> >
> >LGTM, I didn't see anything that would impede applying this series,
> >so
> >Reviewed-by: Chris Wilson 
> >
> >(Goes off to double check...)
> 
> R-b for the series or 1/7 ?

Series, just a few more wishlist items for later.
-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 50/64] drm/i915: Prepare i915_gem_active for annotations

[Chris Wilson]

On Thu, Jul 14, 2016 at 10:32:05AM +0100, Tvrtko Ursulin wrote:
> 
> On 13/07/16 16:58, Chris Wilson wrote:
> >On Wed, Jul 13, 2016 at 04:40:03PM +0100, Tvrtko Ursulin wrote:
> 
> [snip]
> 
> >>>   } else {
> >>>   for (i = 0; i < I915_NUM_ENGINES; i++) {
> >>>   struct drm_i915_gem_request *req;
> >>>
> >>>-  req = obj->last_read[i].request;
> >>>+  req = i915_gem_active_peek(>last_read[i]);
> >>>   if (req == NULL)
> >>>   continue;
> >>>
> >>>-  requests[n++] = i915_gem_request_get(req);
> >>>+  requests[n++] = req;
> >>>   }
> >>>   }
> >>>
> >>>@@ -2383,25 +2386,27 @@ void i915_vma_move_to_active(struct i915_vma *vma,
> >>>  static void
> >>>  i915_gem_object_retire__write(struct drm_i915_gem_object *obj)
> >>>  {
> >>>-  GEM_BUG_ON(!obj->last_write.request);
> >>>-  GEM_BUG_ON(!(obj->active & 
> >>>intel_engine_flag(obj->last_write.request->engine)));
> >>>+  GEM_BUG_ON(!__i915_gem_active_is_busy(>last_write));
> >>>+  GEM_BUG_ON(!(obj->active & 
> >>>intel_engine_flag(i915_gem_active_get_engine(>last_write;
> >>>
> >>>-  i915_gem_request_assign(>last_write.request, NULL);
> >>>+  i915_gem_active_set(>last_write, NULL);
> >>
> >>Aha!
> >
> >Drat. Didn't think I did that...
> >
> >Oh well, no excuses now but to go back in time and make the change
> >earlier. It does get removed eventually!
> 
> Probably not worth it. You can have a special dispensation since I
> am reviewing all the same lines of code patch after patch anyway. :)

Too late, since this patch had to be fixed, I did the earlier fixup as
well.
-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 v3] drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl)

[Chris Wilson]

On Thu, Jul 14, 2016 at 10:12:17AM +0200, Daniel Vetter wrote:
> On Thu, Jul 14, 2016 at 08:04:19AM +0100, Chris Wilson wrote:
> > vGEM buffers are useful for passing data between software clients and
> > hardware renders. By allowing the user to create and attach fences to
> > the exported vGEM buffers (on the dma-buf), the user can implement a
> > deferred renderer and queue hardware operations like flipping and then
> > signal the buffer readiness (i.e. this allows the user to schedule
> > operations out-of-order, but have them complete in-order).
> > 
> > This also makes it much easier to write tightly controlled testcases for
> > dma-buf fencing and signaling between hardware drivers.
> > 
> > v2: Don't pretend the fences exist in an ordered timeline, but allocate
> > a separate fence-context for each fence so that the fences are
> > unordered.
> > v3: Make the debug output more interesting, and so the signaled status.
> > 
> > Testcase: igt/vgem_basic/dmabuf-fence
> > Signed-off-by: Chris Wilson 
> > Cc: Sean Paul 
> > Cc: Zach Reizner 
> > Cc: Gustavo Padovan 
> > Cc: Daniel Vetter 
> > Acked-by: Zach Reizner 
> 
> One thing I completely forgotten: This allows userspace to hang kernel
> drivers. i915 (and other gpu drivers) can recover using hangcheck, but
> dumber drivers (v4l, if that ever happens) probably never except such a
> case. We've had a similar discusion with the userspace fences exposed in
> sw_fence, and decided to move all those ioctl into debugfs. I think we
> should do the same for this vgem-based debugging of implicit sync. Sorry
> for realizing this this late.

One of the very tests I make is to ensure that we recover from such a
hang. I don't see the difference between this any of the other ways
userspace can shoot itself (and others) in the foot.
-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 50/64] drm/i915: Prepare i915_gem_active for annotations

[Tvrtko Ursulin]

On 13/07/16 16:58, Chris Wilson wrote:

On Wed, Jul 13, 2016 at 04:40:03PM +0100, Tvrtko Ursulin wrote:


[snip]


} else {
for (i = 0; i < I915_NUM_ENGINES; i++) {
struct drm_i915_gem_request *req;

-   req = obj->last_read[i].request;
+   req = i915_gem_active_peek(>last_read[i]);
if (req == NULL)
continue;

-   requests[n++] = i915_gem_request_get(req);
+   requests[n++] = req;
}
}

@@ -2383,25 +2386,27 @@ void i915_vma_move_to_active(struct i915_vma *vma,
  static void
  i915_gem_object_retire__write(struct drm_i915_gem_object *obj)
  {
-   GEM_BUG_ON(!obj->last_write.request);
-   GEM_BUG_ON(!(obj->active & 
intel_engine_flag(obj->last_write.request->engine)));
+   GEM_BUG_ON(!__i915_gem_active_is_busy(>last_write));
+   GEM_BUG_ON(!(obj->active & 
intel_engine_flag(i915_gem_active_get_engine(>last_write;

-   i915_gem_request_assign(>last_write.request, NULL);
+   i915_gem_active_set(>last_write, NULL);


Aha!


Drat. Didn't think I did that...

Oh well, no excuses now but to go back in time and make the change
earlier. It does get removed eventually!


Probably not worth it. You can have a special dispensation since I am 
reviewing all the same lines of code patch after patch anyway. :)


Regards,

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

Re: [Intel-gfx] [PATCH 6/7] drm/i915: Move common engine setup into intel_engine_cs.c

[Tvrtko Ursulin]

On 13/07/16 17:04, Chris Wilson wrote:

On Wed, Jul 13, 2016 at 04:03:40PM +0100, Tvrtko Ursulin wrote:

+   /*
+* Catch failures to update intel_engines table when the new engines
+* are added to the driver by a warning and disabling the forgotten
+* engines.
+*/
+   if (WARN_ON(mask != INTEL_INFO(dev_priv)->ring_mask)) {
+   struct intel_device_info *info =
+   (struct intel_device_info *)_priv->info;


I snuck in mkwrite_device_info(), so

if (WARN_ON(mask != INTEL_INFO(dev_priv)->ring_mask))
mkwrite_device_info(dev_priv)->ring_mask = mask;


This part is just code movement, the block you quote exists before this 
series even!


Follow up patch to this series would be easiest then, or a solitary 
precursor if you insist. Dangers of code movement with edits huh? 
(94b4f3ba483ace6dd4a3f881e19cc18bdbafa6ef)


Regards,

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

Re: [Intel-gfx] [PATCH 1/7] drm/i915: unify first-stage engine struct setup

[Tvrtko Ursulin]

On 13/07/16 17:01, Chris Wilson wrote:

On Wed, Jul 13, 2016 at 04:03:35PM +0100, Tvrtko Ursulin wrote:

From: Dave Gordon 

intel_lrc.c has a table of "logical rings" (meaning engines), while
intel_ringbuffer.c has separately open-coded initialisation for each
engine. We can deduplicate this somewhat by using the same first-stage
engine-setup function for both modes.

So here we expose the function that transfers information from the
static table of (all) known engines to the dev_priv->engine array of
engines available on this device (adjusting the names along the way)
and then embed calls to it in both the LRC and the legacy-mode setup.

Signed-off-by: Dave Gordon 


LGTM, I didn't see anything that would impede applying this series,
so
Reviewed-by: Chris Wilson 

(Goes off to double check...)


R-b for the series or 1/7 ?

Regards,

Tvrtko

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

Re: [Intel-gfx] [PATCH v3 2/3] drm/i915/bxt: Fix inadvertent CPU snooping due to incorrect MOCS config

[Yang, Rong R]

> -Original Message-
> From: Deak, Imre
> Sent: Friday, July 1, 2016 21:40
> To: intel-gfx@lists.freedesktop.org
> Cc: Ville Syrjälä ; Chris Wilson  wilson.co.uk>; Yang, Rong R ; Zhao, Yakui
> ; Tamminen, Eero T 
> Subject: [PATCH v3 2/3] drm/i915/bxt: Fix inadvertent CPU snooping due to
> incorrect MOCS config
> 
> Setting a write-back cache policy in the MOCS entry definition also implies
> snooping, which has a considerable overhead. This is unexpected for a few
> reasons:
> - From user-space's point of view since it didn't want a coherent
>   surface (it didn't set the buffer as such via the set caching IOCTL).
> - There is a separate MOCS entry field for snooping (which we never
>   set).
> - This MOCS table is about caching in (e)LLC and there is no (e)LLC on
>   BXT. There is a separate table for L3 cache control.
> 
> Considering the above the current behavior of snooping looks like an
> unintentional side-effect of the WB setting. Changing it to be LLC-UC gets rid
> of the snooping without any ill-effects. For a coherent surface the 
> application
> would use a separate MOCS entry at index 1 and call the set caching IOCTL to
> setup the PTE entries for the corresponding buffer to be snooped. In the
> future we could also add a new MOCS entry for coherent surfaces.
> 
> This resulted in 70% improvement in synthetic texturing benchmarks.
> 
> Kudos to Valtteri Rantala, Eero Tamminen and Michael T Frederick and Ville
> who helped to narrow the source of problem to the kernel and to the
> snooping behaviour in particular.
> 
> With a follow-up change to adjust the 3rd entry value
> igt/gem_mocs_settings is passing after this change.
> 
> v2:
> - Rebase on v2 of patch 1/2.
> v3:
> - Set the entry as LLC uncached instead of PTE-passthrough. This way
>   we also keep snooping disabled, but we also make the cacheability/
>   coherency setting indepent of the PTE which is managed by the
>   kernel. (Chris)

About 20% improvement in OpenCL benchmark luxmark.
Add: Tested-by: Rong R Yang 
 
> CC: Rong R Yang 
> CC: Yakui Zhao 
> CC: Valtteri Rantala 
> CC: Eero Tamminen 
> CC: Michael T Frederick 
> CC: Ville Syrjälä 
> CC: Chris Wilson 
> Signed-off-by: Imre Deak 
> ---
>  drivers/gpu/drm/i915/intel_mocs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_mocs.c
> b/drivers/gpu/drm/i915/intel_mocs.c
> index d36e609..927825f 100644
> --- a/drivers/gpu/drm/i915/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/intel_mocs.c
> @@ -149,8 +149,8 @@ static const struct drm_i915_mocs_entry
> broxton_mocs_table[] = {
> .l3cc_value =L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
>   },
>   {
> -   /* 0x003b */
> -   .control_value = LE_CACHEABILITY(LE_WB) |
> +   /* 0x0039 */
> +   .control_value = LE_CACHEABILITY(LE_UC) |
>  LE_TGT_CACHE(LE_TC_LLC_ELLC) |
>  LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
>  LE_PFM(0) | LE_SCF(0),
> --
> 2.5.0

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

[Intel-gfx] [PULL] topic/drm-misc

[Daniel Vetter]

Hi Dave,

I recovered dri-devel backlog from my vacation, more misc stuff:
- of_put_node fixes from Peter Chen (not all yet)
- more patches from Gustavo to use kms-native drm_crtc_vblank_* funcs
- docs sphinxification from Lukas Wunner
- bunch of fixes all over from Dan Carpenter
- more follow up work from Chris register/unregister rework in various
  places
- vgem dma-buf export (for writing testcases)
- small things all over from tons of different people

This is just the delta against the previous pull request, pls take them
both.

Cheers, Daniel


The following changes since commit 2a3467063ae3b17264578626dec2377dd48cd1c3:

  Merge tag 'mediatek-drm-2016-06-20' of git://git.pengutronix.de/git/pza/linux 
into drm-next (2016-06-24 13:16:07 +1000)

are available in the git repository at:

  git://anongit.freedesktop.org/drm-intel tags/topic/drm-misc-2016-07-14

for you to fetch changes up to 01d3434a565ada5ca084c68ec1e087ada5a7b157:

  drm: Don't overwrite user ioctl arg unless requested (2016-07-14 10:12:50 
+0200)


Alexey Khoroshilov (1):
  drm_aux-dev: fix error handling in drm_dp_aux_dev_init()

Benjamin Herrenschmidt (1):
  drm: Fix broken use of _PAGE_NO_CACHE on powerpc

Bhaktipriya Shridhar (1):
  drm/qxl: Remove deprecated create_singlethread_workqueue

Chris Wilson (8):
  drm/vgem: Fix mmaping
  drm/vgem: Enable dmabuf interface for export
  drm: Unexport drm_connector_register_all()
  drm: Do a full device unregister when unplugging
  drm/udl: Unplugging a device now unregisters it
  drm: Restore double clflush on the last partial cacheline
  drm/vgem: Use PAGE_KERNEL in place of x86-specific PAGE_KERNEL_IO
  drm: Don't overwrite user ioctl arg unless requested

Dan Carpenter (3):
  drm/mediatek/mtk_mipi_tx: checking the wrong variable
  qxl: check for kmap failures
  qxl: silence uninitialized variable warning

Daniel Vetter (1):
  drm: Resurrect atomic rmfb code

Frank Binns (2):
  drm: fix some spelling mistakes
  drm/vmwgfx: Stop checking minor type directly

Gustavo Padovan (8):
  drm: make drm_vblank_count_and_time() static
  drm/armada: use drm_crtc_handle_vblank()
  drm/atmel: use drm_crtc_handle_vblank()
  drm/nouveau: use drm_crtc_handle_vblank()
  drm/rcar-du: use drm_crtc_handle_vblank()
  drm/tilcdc: use drm_crtc_handle_vblank()
  MAINTAINERS: add entry for the Sync File Framework
  dma-buf/sync_file: improve Kconfig description for Sync Files

Lukas Wunner (16):
  drm/nouveau: Don't leak runtime pm ref on driver unload
  drm/nouveau: Forbid runtime pm on driver unload
  drm/radeon: Don't leak runtime pm ref on driver unload
  drm/radeon: Don't leak runtime pm ref on driver load
  drm/radeon: Forbid runtime pm on driver unload
  drm/amdgpu: Don't leak runtime pm ref on driver unload
  drm/amdgpu: Don't leak runtime pm ref on driver load
  drm/amdgpu: Forbid runtime pm on driver unload
  drm: Add helpers to turn off CRTCs
  drm/nouveau: Turn off CRTCs on driver unload
  drm/radeon: Turn off CRTCs on driver unload
  drm/amdgpu: Turn off CRTCs on driver unload
  drm: Use helper to turn off CRTC
  drm/i2c/ch7006: Use helper to turn off CRTC
  drm/nouveau/dispnv04: Use helper to turn off CRTC
  vga_switcheroo: Sphinxify docs

Masanari Iida (1):
  drm: Fix a typo in drm_ioctl.c

Michel Dänzer (1):
  drm: Only handle _DRM_VBLANK_NEXTONMISS once

Peter Chen (5):
  gpu: drm: sti_compositor: add missing of_node_put after calling 
of_parse_phandle
  gpu: drm: sti_vdo: add missing of_node_put after calling of_parse_phandle
  gpu: drm: sti_hqvdp: add missing of_node_put after calling 
of_parse_phandle
  gpu: drm: sti_vtg: add missing of_node_put after calling of_parse_phandle
  gpu: drm: rockchip_drm_drv: add missing of_node_put after calling 
of_parse_phandle

Thierry Reding (2):
  drm/qxl: Remove dead code
  drm/dsi: Make set_tear_scanline command consistent

Tobias Jakobi (1):
  drm/exynos: make fbdev support really optional

Xinliang Liu (1):
  drm/hisilicon: Fix ADE vblank on/off handling

 Documentation/gpu/drm-internals.rst |   4 +-
 Documentation/gpu/vga-switcheroo.rst|   8 +-
 MAINTAINERS |  11 ++
 drivers/dma-buf/Kconfig |  15 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  |   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |  12 +-
 drivers/gpu/drm/armada/armada_crtc.c|   2 +-
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c  |   2 +-
 drivers/gpu/drm/drm_atomic.c|  66 +++
 drivers/gpu/drm/drm_cache.c |   1 +
 drivers/gpu/drm/drm_crtc.c  |  78 +---
 drivers/gpu/drm/drm_crtc_internal.h |   1 +
 drivers/gpu/drm/drm_dp_aux_dev.c 

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

[Daniel Vetter]

Hi Dave,

drm-intel-next-2016-07-11:
- select igt testing depencies for CONFIG_DRM_I915_DEBUG (Chris)
- track outputs in crtc state and clean up all our ad-hoc connector/encoder
  walking in modest code (Ville)
- demidlayer drm_device/drm_i915_private (Chris Wilson)
- thundering herd fix from Chris Wilson, with lots of help from Tvrtko Ursulin
- piles of assorted clean and fallout from the thundering herd fix
- documentation and more tuning for waitboosting (Chris)
- pooled EU support on bxt (Arun Siluvery)
- bxt support is no longer considered prelimary!
- ring/engine vfunc cleanup from Tvrtko
- introduce intel_wait_for_register helper (Chris)
- opregion updates (Jani Nukla)
- tuning and fixes for wait_for macros (Tvrkto)
- more kabylake pci ids (Rodrigo)
- pps cleanup and fixes for bxt (Imre)
- move sink crc support over to atomic state (Maarten)
- fix up async fbdev init ordering (Chris)
- fbc fixes from Paulo and Chris

Final feature pull request for 4.8.

Cheers, Daniel


The following changes since commit 2a3467063ae3b17264578626dec2377dd48cd1c3:

  Merge tag 'mediatek-drm-2016-06-20' of git://git.pengutronix.de/git/pza/linux 
into drm-next (2016-06-24 13:16:07 +1000)

are available in the git repository at:

  git://anongit.freedesktop.org/drm-intel tags/drm-intel-next-2016-07-11

for you to fetch changes up to 0b2c0582f1570bfc95aa9ac1cd340a215d8e8335:

  drm/i915: Update DRIVER_DATE to 20160711 (2016-07-11 09:18:31 +0200)


- select igt testing depencies for CONFIG_DRM_I915_DEBUG (Chris)
- track outputs in crtc state and clean up all our ad-hoc connector/encoder
  walking in modest code (Ville)
- demidlayer drm_device/drm_i915_private (Chris Wilson)
- thundering herd fix from Chris Wilson, with lots of help from Tvrtko Ursulin
- piles of assorted clean and fallout from the thundering herd fix
- documentation and more tuning for waitboosting (Chris)
- pooled EU support on bxt (Arun Siluvery)
- bxt support is no longer considered prelimary!
- ring/engine vfunc cleanup from Tvrtko
- introduce intel_wait_for_register helper (Chris)
- opregion updates (Jani Nukla)
- tuning and fixes for wait_for macros (Tvrkto)
- more kabylake pci ids (Rodrigo)
- pps cleanup and fixes for bxt (Imre)
- move sink crc support over to atomic state (Maarten)
- fix up async fbdev init ordering (Chris)
- fbc fixes from Paulo and Chris


Chris Wilson (149):
  drm/i915: Extract checking for backing struct pages to a helper
  drm/i915: pwrite/pread do not require obj->base.filp, just pages
  drm/i915: use ORIGIN_CPU for frontbuffer invalidation on WC mmaps
  drm/i915/fbdev: Perform async fbdev initialisation much later
  drm/i915/fbdev: Limit the global async-domain synchronization
  drm/i915/fbdev: Flush mode configuration before lastclose
  drm/i915/gvt: Mark i915.enable_gvt as false if loading fails
  drm/i915: Move panel's backlight setup next to panel init
  drm/i915: Move registration actions to connector->late_register
  drm/i915: Move backlight registration to connector registration
  drm/i915: Move connector registration to driver registration
  drm/i915: Register debugfs interface last
  drm/i915: Demidlayer driver loading
  drm/i915: Demidlayer driver unloading
  drm/i915: Remove redundant drm_connector_register_all()
  drm/i915: Start exploiting drm_device subclassing
  drm/i915: Merge i915_dma.c into i915_drv.c
  drm/i915: Remove user controllable DRM_ERROR for i915_getparam()
  drm/i915: Remove user controllable DRM_ERROR for 
intel_get_pipe_from_crtc_id()
  drm/i915: Split out the PCI driver interface to i915_pci.c
  drm/i915: Move module init/exit to i915_pci.c
  drm/i915: Skip idling an idle engine
  drm/i915: Move legacy kernel context pinning to intel_ringbuffer.c
  drm/i915: Treat kernel context as initialised
  drm/i915: Mark all default contexts as uninitialised after context loss
  drm/i915: No need to wait for idle on L3 remap
  drm/i915: Split idling from forcing context switch
  drm/i915: Only switch to default context when evicting from GGTT
  drm/i915: Remove request->reset_counter
  Revert "drm/i915: Use atomic commits for legacy page_flips"
  drm/i915: Use a hybrid scheme for fast register waits
  drm/i915: Convert sandybridge_pcode_*() to use intel_wait_for_register()
  drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()
  drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()
  drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()
  drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()
  drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()
  drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()
  drm/i915: 

Re: [Intel-gfx] [PATCH 1/8] drm/i915: Flush GT idle status upon reset

[Chris Wilson]

On Thu, Jul 14, 2016 at 10:53:20AM +0300, Joonas Lahtinen wrote:
> On ke, 2016-07-13 at 09:10 +0100, Chris Wilson wrote:
> > Upon resetting the GPU, we force the engines to be idle by clearing
> > their request lists. However, I neglected to clear the GT active status
> > and so the next request following the reset was not marking the device
> > as busy again. (We had to wait until any outstanding retire worker
> > finally ran and cleared the active status.)
> > 
> > Fixes: 67d97da34917 ("drm/i915: Only start retire worker when idle")
> > Testcase: igt/pm_rps/reset
> > Signed-off-by: Chris Wilson 
> > Cc: Joonas Lahtinen 
> 
> No FDO bug associated?

Unlikely, I had to rewrite the test case to hit it reliably.
-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 1/8] drm/i915: Flush GT idle status upon reset

[Joonas Lahtinen]

On ke, 2016-07-13 at 09:10 +0100, Chris Wilson wrote:
> Upon resetting the GPU, we force the engines to be idle by clearing
> their request lists. However, I neglected to clear the GT active status
> and so the next request following the reset was not marking the device
> as busy again. (We had to wait until any outstanding retire worker
> finally ran and cleared the active status.)
> 
> Fixes: 67d97da34917 ("drm/i915: Only start retire worker when idle")
> Testcase: igt/pm_rps/reset
> Signed-off-by: Chris Wilson 
> Cc: Joonas Lahtinen 

No FDO bug associated?

Reviewed-by: Joonas Lahtinen 

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index adeca0ec4cfb..9d8c26f42dee 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3169,6 +3169,8 @@ static void i915_gem_reset_engine_cleanup(struct 
> intel_engine_cs *engine)
>   }
>  
>   intel_ring_init_seqno(engine, engine->last_submitted_seqno);
> +
> + engine->i915->gt.active_engines &= ~intel_engine_flag(engine);
>  }
>  
>  void i915_gem_reset(struct drm_device *dev)
> @@ -3186,6 +3188,7 @@ void i915_gem_reset(struct drm_device *dev)
>  
>   for_each_engine(engine, dev_priv)
>   i915_gem_reset_engine_cleanup(engine);
> + mod_delayed_work(dev_priv->wq, _priv->gt.idle_work, 0);
>  
>   i915_gem_context_reset(dev);
>  
-- 
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] ✗ Ro.CI.BAT: failure for drm/i915: set proper N/M in modeset

[Patchwork]

== Series Details ==

Series: drm/i915: set proper N/M in modeset
URL   : https://patchwork.freedesktop.org/series/9857/
State : failure

== Summary ==

Series 9857v1 drm/i915: set proper N/M in modeset
http://patchwork.freedesktop.org/api/1.0/series/9857/revisions/1/mbox

Test drv_module_reload_basic:
dmesg-warn -> SKIP   (fi-skl-i5-6260u)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
pass   -> DMESG-WARN (ro-skl3-i5-6260u)
Test prime_self_import:
Subgroup basic-with_fd_dup:
pass   -> INCOMPLETE (ro-byt-n2820)
Subgroup basic-with_one_bo_two_files:
pass   -> INCOMPLETE (ro-byt-n2820)
Subgroup basic-with_two_bos:
pass   -> INCOMPLETE (ro-byt-n2820)

fi-kbl-qkkr  total:237  pass:174  dwarn:27  dfail:2   fail:7   skip:27 
fi-skl-i5-6260u  total:237  pass:189  dwarn:26  dfail:2   fail:7   skip:13 
fi-skl-i7-6700k  total:237  pass:200  dwarn:2   dfail:0   fail:9   skip:26 
fi-snb-i7-2600   total:237  pass:188  dwarn:0   dfail:0   fail:9   skip:40 
ro-bdw-i5-5250u  total:237  pass:210  dwarn:2   dfail:0   fail:9   skip:16 
ro-bdw-i7-5600u  total:237  pass:195  dwarn:2   dfail:0   fail:9   skip:31 
ro-bsw-n3050 total:217  pass:170  dwarn:0   dfail:0   fail:4   skip:42 
ro-byt-n2820 total:237  pass:174  dwarn:0   dfail:0   fail:7   skip:38 
ro-hsw-i3-4010u  total:237  pass:199  dwarn:5   dfail:2   fail:7   skip:24 
ro-hsw-i7-4770r  total:237  pass:204  dwarn:0   dfail:0   fail:9   skip:24 
ro-ilk-i7-620lm  total:237  pass:166  dwarn:0   dfail:0   fail:8   skip:63 
ro-ilk1-i5-650   total:232  pass:166  dwarn:0   dfail:0   fail:8   skip:58 
ro-ivb-i7-3770   total:237  pass:197  dwarn:0   dfail:0   fail:7   skip:33 
ro-skl3-i5-6260u total:237  pass:212  dwarn:4   dfail:0   fail:9   skip:12 
ro-snb-i7-2620M  total:237  pass:187  dwarn:0   dfail:0   fail:9   skip:41 
ro-bdw-i7-5557U failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1490/

5cd2699 drm-intel-nightly: 2016y-07m-13d-14h-43m-27s UTC integration manifest
9ea6eab drm/i915: set proper N/M in modeset

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

[Intel-gfx] [PATCH] drm/i915: set proper N/M in modeset

[libin . yang]

From: Libin Yang 

When modeset occurs and the LS_CLK is set to some
special values in DP mode, the N/M need to be set
manually if audio is playing.

Also, the patch applies
commit 7e8275c2f2bb ("drm/i915: set proper N/CTS in modeset")
to APL platform.

Signed-off-by: Libin Yang 
---
 drivers/gpu/drm/i915/i915_reg.h|   6 ++
 drivers/gpu/drm/i915/intel_audio.c | 116 -
 2 files changed, 108 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8bfde75..2f9d00e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7351,6 +7351,12 @@ enum {
 #define _HSW_AUD_CONFIG_B  0x65100
 #define HSW_AUD_CFG(pipe)  _MMIO_PIPE(pipe, _HSW_AUD_CONFIG_A, 
_HSW_AUD_CONFIG_B)
 
+#define _HSW_AUD_M_CTS_ENABLE_A0x65028
+#define _HSW_AUD_M_CTS_ENABLE_B0x65128
+#define HSW_AUD_M_CTS_ENABLE(pipe) _MMIO_PIPE(pipe, 
_HSW_AUD_M_CTS_ENABLE_A, _HSW_AUD_M_CTS_ENABLE_B)
+#define   AUD_M_CTS_M_VALUE_INDEX  (1 << 21)
+#define   AUD_M_CTS_M_PROG_ENABLE  (1 << 20)
+
 #define _HSW_AUD_MISC_CTRL_A   0x65010
 #define _HSW_AUD_MISC_CTRL_B   0x65110
 #define HSW_AUD_MISC_CTRL(pipe)_MMIO_PIPE(pipe, 
_HSW_AUD_MISC_CTRL_A, _HSW_AUD_MISC_CTRL_B)
diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index 6700a7b..e2e4d4b 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -98,6 +98,22 @@ static const struct {
{ 192000, TMDS_297M, 20480, 247500 },
 };
 
+#define LC_533M 533250
+#define LC_148M 148500
+static const struct {
+   int sample_rate;
+   int clock;
+   int n;
+   int m;
+} aud_nm[] = {
+   {48000, LC_533M, 88875, 4096},
+   {44100, LC_533M, 148125, 6272},
+   {32000, LC_533M, 266625, 8192},
+   {48000, LC_148M, 12375, 2048},
+   {44100, LC_148M, 20625, 3136},
+   {32000, LC_148M, 37125, 4096},
+};
+
 /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
 static u32 audio_config_hdmi_pixel_clock(const struct drm_display_mode 
*adjusted_mode)
 {
@@ -121,20 +137,50 @@ static u32 audio_config_hdmi_pixel_clock(const struct 
drm_display_mode *adjusted
return hdmi_audio_clock[i].config;
 }
 
-static int audio_config_get_n(const struct drm_display_mode *mode, int rate)
+static int audio_config_get_n(struct intel_crtc *crtc,
+ const struct drm_display_mode *mode, int rate)
+{
+   int i;
+
+   if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI)) {
+   for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
+   if ((rate == aud_ncts[i].sample_rate) &&
+   (mode->clock == aud_ncts[i].clock)) {
+   return aud_ncts[i].n;
+   }
+   }
+   }
+
+   if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP)) {
+   for (i = 0; i < ARRAY_SIZE(aud_nm); i++) {
+   if ((rate == aud_nm[i].sample_rate) &&
+   (mode->clock == aud_nm[i].clock)) {
+   return aud_nm[i].n;
+   }
+   }
+   }
+   return 0;
+}
+
+static int audio_config_get_m(struct intel_crtc *crtc,
+ const struct drm_display_mode *mode, int rate)
 {
int i;
 
-   for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
-   if ((rate == aud_ncts[i].sample_rate) &&
-   (mode->clock == aud_ncts[i].clock)) {
-   return aud_ncts[i].n;
+   if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP)) {
+   for (i = 0; i < ARRAY_SIZE(aud_nm); i++) {
+   if ((rate == aud_nm[i].sample_rate) &&
+   (mode->clock == aud_nm[i].clock)) {
+   return aud_nm[i].m;
+   }
}
}
+
return 0;
 }
 
-static uint32_t audio_config_setup_n_reg(int n, uint32_t val)
+static uint32_t audio_config_setup_n_reg(struct intel_crtc *crtc,
+int n, uint32_t val)
 {
int n_low, n_up;
uint32_t tmp = val;
@@ -145,6 +191,23 @@ static uint32_t audio_config_setup_n_reg(int n, uint32_t 
val)
tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) |
(n_low << AUD_CONFIG_LOWER_N_SHIFT) |
AUD_CONFIG_N_PROG_ENABLE);
+   if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP))
+   tmp |= AUD_CONFIG_N_VALUE_INDEX;
+   return tmp;
+}
+
+static uint32_t audio_config_setup_m_reg(struct intel_crtc *crtc,
+int m, uint32_t val)
+{
+   uint32_t tmp = val;
+
+   if 

[Intel-gfx] [PATCH v3] drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl)

[Chris Wilson]

vGEM buffers are useful for passing data between software clients and
hardware renders. By allowing the user to create and attach fences to
the exported vGEM buffers (on the dma-buf), the user can implement a
deferred renderer and queue hardware operations like flipping and then
signal the buffer readiness (i.e. this allows the user to schedule
operations out-of-order, but have them complete in-order).

This also makes it much easier to write tightly controlled testcases for
dma-buf fencing and signaling between hardware drivers.

v2: Don't pretend the fences exist in an ordered timeline, but allocate
a separate fence-context for each fence so that the fences are
unordered.
v3: Make the debug output more interesting, and so the signaled status.

Testcase: igt/vgem_basic/dmabuf-fence
Signed-off-by: Chris Wilson 
Cc: Sean Paul 
Cc: Zach Reizner 
Cc: Gustavo Padovan 
Cc: Daniel Vetter 
Acked-by: Zach Reizner 
---
 drivers/gpu/drm/vgem/Makefile |   2 +-
 drivers/gpu/drm/vgem/vgem_drv.c   |  34 +++
 drivers/gpu/drm/vgem/vgem_drv.h   |  16 +++
 drivers/gpu/drm/vgem/vgem_fence.c | 207 ++
 include/uapi/drm/vgem_drm.h   |  62 
 5 files changed, 320 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/vgem/vgem_fence.c
 create mode 100644 include/uapi/drm/vgem_drm.h

diff --git a/drivers/gpu/drm/vgem/Makefile b/drivers/gpu/drm/vgem/Makefile
index 3f4c7b842028..bfcdea1330e6 100644
--- a/drivers/gpu/drm/vgem/Makefile
+++ b/drivers/gpu/drm/vgem/Makefile
@@ -1,4 +1,4 @@
 ccflags-y := -Iinclude/drm
-vgem-y := vgem_drv.o
+vgem-y := vgem_drv.o vgem_fence.o
 
 obj-$(CONFIG_DRM_VGEM) += vgem.o
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 29c2aab3c1a7..c15bafb06665 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -83,6 +83,34 @@ static const struct vm_operations_struct vgem_gem_vm_ops = {
.close = drm_gem_vm_close,
 };
 
+static int vgem_open(struct drm_device *dev, struct drm_file *file)
+{
+   struct vgem_file *vfile;
+   int ret;
+
+   vfile = kzalloc(sizeof(*vfile), GFP_KERNEL);
+   if (!vfile)
+   return -ENOMEM;
+
+   file->driver_priv = vfile;
+
+   ret = vgem_fence_open(vfile);
+   if (ret) {
+   kfree(vfile);
+   return ret;
+   }
+
+   return 0;
+}
+
+static void vgem_preclose(struct drm_device *dev, struct drm_file *file)
+{
+   struct vgem_file *vfile = file->driver_priv;
+
+   vgem_fence_close(vfile);
+   kfree(vfile);
+}
+
 /* ioctls */
 
 static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
@@ -164,6 +192,8 @@ unref:
 }
 
 static struct drm_ioctl_desc vgem_ioctls[] = {
+   DRM_IOCTL_DEF_DRV(VGEM_FENCE_ATTACH, vgem_fence_attach_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(VGEM_FENCE_SIGNAL, vgem_fence_signal_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
 };
 
 static int vgem_mmap(struct file *filp, struct vm_area_struct *vma)
@@ -271,9 +301,12 @@ static int vgem_prime_mmap(struct drm_gem_object *obj,
 
 static struct drm_driver vgem_driver = {
.driver_features= DRIVER_GEM | DRIVER_PRIME,
+   .open   = vgem_open,
+   .preclose   = vgem_preclose,
.gem_free_object_unlocked   = vgem_gem_free_object,
.gem_vm_ops = _gem_vm_ops,
.ioctls = vgem_ioctls,
+   .num_ioctls = ARRAY_SIZE(vgem_ioctls),
.fops   = _driver_fops,
 
.dumb_create= vgem_gem_dumb_create,
@@ -328,5 +361,6 @@ module_init(vgem_init);
 module_exit(vgem_exit);
 
 MODULE_AUTHOR("Red Hat, Inc.");
+MODULE_AUTHOR("Intel Corporation");
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_LICENSE("GPL and additional rights");
diff --git a/drivers/gpu/drm/vgem/vgem_drv.h b/drivers/gpu/drm/vgem/vgem_drv.h
index 988cbaae7588..1f8798ad329c 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.h
+++ b/drivers/gpu/drm/vgem/vgem_drv.h
@@ -32,9 +32,25 @@
 #include 
 #include 
 
+#include 
+
+struct vgem_file {
+   struct idr fence_idr;
+   struct mutex fence_mutex;
+};
+
 #define to_vgem_bo(x) container_of(x, struct drm_vgem_gem_object, base)
 struct drm_vgem_gem_object {
struct drm_gem_object base;
 };
 
+int vgem_fence_open(struct vgem_file *file);
+int vgem_fence_attach_ioctl(struct drm_device *dev,
+   void *data,
+   struct drm_file *file);
+int vgem_fence_signal_ioctl(struct drm_device *dev,
+   void *data,
+   struct drm_file *file);
+void vgem_fence_close(struct vgem_file *file);
+
 #endif
diff --git a/drivers/gpu/drm/vgem/vgem_fence.c