Re: [Intel-gfx] [PATCH 5/5] drm: Check plane src coordinates correctly during page flip for atomic drivers

[Daniel Vetter]

On Fri, Oct 16, 2015 at 05:40:59PM +0100, Tvrtko Ursulin wrote:
> 
> On 16/10/15 17:27, Matt Roper wrote:
> >On Thu, Oct 15, 2015 at 08:40:02PM +0300, ville.syrj...@linux.intel.com 
> >wrote:
> >>From: Ville Syrjälä 
> >>
> >>Instead of relying on the old crtc-{x,y,mode} gunk, dig out the primary
> >>plane coordinates from the plane state when checking them against the
> >>new framebuffer during page flip.
> >>
> >>Cc: Matt Roper 
> >>Cc: Tvrtko Ursulin 
> >>Cc: Daniel Vetter 
> >>Signed-off-by: Ville Syrjälä 
> >
> >For the series:
> >
> >Reviewed-by: Matt Roper 

Pulled all 5 into drm-misc, thanks.

> >I also confirmed that the i-g-t test I wrote here:
> >http://lists.freedesktop.org/archives/intel-gfx/2015-October/077394.html
> >now passes with your patch series, so I believe Tvrtko's original bug
> >report should be fixed.
> 
> Oh I did not realize this series is about this, perhaps because I did not
> see 1/5 which maybe had some more obvious clues. :)
> 
> Great, that means if we decide to merge "drm/i915: Consider plane rotation
> when calculating stride in skl_do_mmio_flip" and "kms_rotation_crc: Exercise
> page flips with 90 degree rotation" we would have working rotated legacy
> page flip with a test case.

Matt, can you please push these igt patches and double-check that we have
subtests to catch the viewport confusion here?

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

Re: [Intel-gfx] [Regression report] Weekly regression report WW42

[Andreas Schildbach]

On 10/16/2015 07:23 PM, jairo.daniel.miramontes.ca...@linux.intel.com wrote:

> | 92237 | Horrible noise (audio) via DisplayPort [regre | 2015-10-02 | No 
> |

This issue actually IS bisected:

commit fdbc3b1f639bb2cbfb32c612b2699e0ba373317d
Author: Jani Nikula 
Date: Tue Nov 12 17:10:13 2013 +0200

drm/i915/dp: set sink to power down mode on dp disable

FWIW, here is the full bisect log:

$ git bisect log
# bad: [52cd2b0342665668e7d3806d4a0b2ff837651690] UBUNTU: Ubuntu-3.13.0-0.2
# good: [4f57e47bab90529e40d11878ba8b5f429cfa1d95] UBUNTU:
Ubuntu-3.12.0-8.16
git bisect start 'Ubuntu-3.13.0-0.2' 'Ubuntu-3.12.0-8.16'
# good: [5e01dc7b26d9f24f39abace5da98ccbd6a5ceb52] Linux 3.12
git bisect good 5e01dc7b26d9f24f39abace5da98ccbd6a5ceb52
# good: [5cbb3d216e2041700231bcfc383ee5f8b7fc8b74] Merge branch 'akpm'
(patches from Andrew Morton)
git bisect good 5cbb3d216e2041700231bcfc383ee5f8b7fc8b74
# good: [d8fe4acc88da8fbbe360b6592c9d0abbb85117dc] Merge branch 'akpm'
(patch-bomb from Andrew Morton)
git bisect good d8fe4acc88da8fbbe360b6592c9d0abbb85117dc
# good: [73d75ba99e3bdd627275afd3fe48cc933723084b] Merge tag
'sound-fix-3.13-rc1' of
git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
git bisect good 73d75ba99e3bdd627275afd3fe48cc933723084b
# good: [e6d69a60b77a6ea8d5f9d41765c7571bb8d45531] Merge branch 'next'
of git://git.infradead.org/users/vkoul/slave-dma
git bisect good e6d69a60b77a6ea8d5f9d41765c7571bb8d45531
# good: [d2c2ad54c485e7ebca5c0b7e4a7b2c56103fda38] Merge
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
git bisect good d2c2ad54c485e7ebca5c0b7e4a7b2c56103fda38
# bad: [26b265cd29dde56bf0901c421eabc7ae815f38c4] Merge
git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6
git bisect bad 26b265cd29dde56bf0901c421eabc7ae815f38c4
# bad: [aecde27c4fc4939f7c16ae13645f896438190567] Merge branch
'drm-fixes' of git://people.freedesktop.org/~airlied/linux
git bisect bad aecde27c4fc4939f7c16ae13645f896438190567
# good: [b0e3636f656c98bdeded5aaa78601e3256b18d6d] Merge branch
'for-next' of
git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending
git bisect good b0e3636f656c98bdeded5aaa78601e3256b18d6d
# bad: [cf969677945e6e19810d616873617320da002e32] Merge tag
'drm-intel-fixes-2013-11-20' of
git://people.freedesktop.org/~danvet/drm-intel into drm-fixes
git bisect bad cf969677945e6e19810d616873617320da002e32
# good: [7272c9d2286525d4c6bce788243cf2b6f306d15c] drm/radeon: hook up
backlight functions for CI and KV family.
git bisect good 7272c9d2286525d4c6bce788243cf2b6f306d15c
# bad: [7a495cfd9b5f82c40608f26fe523dc9e8533dc14] drm/i915/tv: add
->get_config callback
git bisect bad 7a495cfd9b5f82c40608f26fe523dc9e8533dc14
# bad: [f69e515699d8e9b1c25dcfe1c4c6f435087495d2] i915: Use 120MHz LVDS
SSC clock for gen5/gen6/gen7
git bisect bad f69e515699d8e9b1c25dcfe1c4c6f435087495d2
# bad: [7bd40c16ccb2cb6877dd00b0e66249c171e6fa43] x86/early quirk: use
gen6 stolen detection for VLV
git bisect bad 7bd40c16ccb2cb6877dd00b0e66249c171e6fa43
# bad: [fdbc3b1f639bb2cbfb32c612b2699e0ba373317d] drm/i915/dp: set sink
to power down mode on dp disable
git bisect bad fdbc3b1f639bb2cbfb32c612b2699e0ba373317d
# first bad commit: [fdbc3b1f639bb2cbfb32c612b2699e0ba373317d]
drm/i915/dp: set sink to power down mode on dp disable
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/fb-helper: Fix fb refcounting in pan_display_atomic

[Daniel Vetter]

In

commit bbb1e52402b2a288b09ae37e8182599931c7e9df
Author: Rob Clark 
Date:   Tue Aug 25 15:35:58 2015 -0400

drm/fb-helper: atomic restore_fbdev_mode()..

we've forgotten to do the plane->old_fb refcount dance for
pan_display_atomic, which can result in refcount leaks if the current
configuration is not from fbcon. Which apparently can happen when
vt-switching - fbcon does a pan first before a set_par.

OCD-align function parameters while at it.

v2: Actually git add the OCD.

Cc: Rob Clark 
Cc: Rodrigo Vivi 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92483
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_fb_helper.c | 29 ++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 0ac0fcc9b0d2..e673c13c7391 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1231,7 +1231,7 @@ int drm_fb_helper_set_par(struct fb_info *info)
 EXPORT_SYMBOL(drm_fb_helper_set_par);
 
 static int pan_display_atomic(struct fb_var_screeninfo *var,
-   struct fb_info *info)
+ struct fb_info *info)
 {
struct drm_fb_helper *fb_helper = info->par;
struct drm_device *dev = fb_helper->dev;
@@ -1249,6 +1249,8 @@ retry:
 
mode_set = _helper->crtc_info[i].mode_set;
 
+   mode_set->crtc->primary->old_fb = mode_set->crtc->primary->fb;
+
mode_set->x = var->xoffset;
mode_set->y = var->yoffset;
 
@@ -1264,13 +1266,34 @@ retry:
info->var.xoffset = var->xoffset;
info->var.yoffset = var->yoffset;
 
-   return 0;
 
 fail:
+   for(i = 0; i < fb_helper->crtc_count; i++) {
+   struct drm_mode_set *mode_set;
+   struct drm_plane *plane;
+
+   mode_set = _helper->crtc_info[i].mode_set;
+   plane = mode_set->crtc->primary;
+
+   if (ret == 0) {
+   struct drm_framebuffer *new_fb = plane->state->fb;
+
+   if (new_fb)
+   drm_framebuffer_reference(new_fb);
+   plane->fb = new_fb;
+   plane->crtc = plane->state->crtc;
+
+   if (plane->old_fb)
+   drm_framebuffer_unreference(plane->old_fb);
+   }
+   plane->old_fb = NULL;
+   }
+
if (ret == -EDEADLK)
goto backoff;
 
-   drm_atomic_state_free(state);
+   if (ret != 0)
+   drm_atomic_state_free(state);
 
return ret;
 
-- 
2.5.1

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

Re: [Intel-gfx] [PATCH] drm/i915: Report context GTT size

[Chris Wilson]

On Fri, Oct 16, 2015 at 05:34:55PM +0100, Tvrtko Ursulin wrote:
> I have a slight unknown relating to how long would this ABI be
> useful. If things are moving towards SVM, and the fact pre-gen8
> platforms can already use get_aperture, would that make it a bit
> short lived?

Optimist. It is certainly a better fit than the cascade of get param and
hardcoded sizes based on gen (which ignore differences between kernels)
which a fallback to get_aperture. Now although I can't just drop the old
code (because I need to support kernels without this interface), it does
make that code simpler and more robust against any future change.
 
> And get_aperture would even be a better place for this if PPGTT size
> will always be the same for all clients.

Hmm, I'm regretting my earlier push for extending get_aperture.
Constants like this would be better off in GET_PARAM (or
CONTEXT_PER_PARAM as applicable), and only using get_aperture where we
want the side-effect of scanning for available space (e.g. the extra
stolen information would be more useful in get_aperture as we there do
want to know the largest object we can allocate given current usage, but
we may also want to stick the total amount of stolen in a param for
convenience).

[snip]
-Chris

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

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

[jairo . daniel . miramontes . caton]

WW42 Regression report.

This week's regressions
+---+---+++
| BugId | Summary   | Created on | Bisect |
+---+---+++
| 92355 | [SKL Regression] igt/kms_fbc_crc cause DUT cr | 2015-10-09 | No |
| 92414 | [Intel-gfx] As of kernel 4.3-rc1 system will  | 2015-10-10 | Yes|
| 92483 | [regression] Can only go to console (fbcon/fb | 2015-10-15 | No |
| 92502 | [SKL] [Regression] igt/kms_flip/2x-flip-vs-ex | 2015-10-16 | No |
+---+---+++


Older regressions
+---+---+++
| BugId | Summary   | Created on | Bisect |
+---+---+++
| 56834 | [ivb DP dual link DVI dongle regression] [drm | 2012-11-07 | No |
| 72782 | [945GM bisected] screen blank on S3 resume on | 2013-12-17 | Yes|
| 80896 | [BDW Regression]dmesg error "<3>[1.954836 | 2014-07-04 | No |
| 81537 | [snb dp regression] dp retry forever due to s | 2014-07-19 | No |
| 84855 | [ILK regression]igt kms_rotation_crc/sprite-r | 2014-10-10 | No |
| 84974 | [VLV eDP-LVDS bisected] powerdomains: Screen  | 2014-10-14 | Yes|
| 87131 | [PNV regression] igt/gem_exec_lut_handle take | 2014-12-09 | No |
| 87480 | [SNB/IVB/HSW/BYT bisected]igt/kms_force_conne | 2014-12-19 | Yes|
| 87662 | [ALL 3.18 Bisected] DVI --rotation inverted c | 2014-12-24 | Yes|
| 87725 | [BDW Bisected] OglBatch7 performance reduced  | 2014-12-26 | Yes|
| 87726 | [BDW Bisected] OglDrvCtx performance reduced  | 2014-12-26 | Yes|
| 88012 | [bisected BYT] complete freeze after: drm/i91 | 2015-01-04 | Yes|
| 88124 | i915: regression: after DP connected monitor  | 2015-01-06 | No |
| 88325 | screen brightness regression on resume| 2015-01-12 | No |
| 88439 | [BDW Bisected]igt/gem_reloc_vs_gpu/forked-fau | 2015-01-15 | Yes|
| 89334 | [945 regression] 4.0-rc1 kernel GPU hang:  ec | 2015-02-26 | No |
| 89629 | [i965 regression]igt/kms_rotation_crc/sprite- | 2015-03-18 | No |
| 89632 | [i965 regression]igt/kms_universal_plane/univ | 2015-03-18 | No |
| 89728 | [HSW/BDW/BSW/SKL bisected] igt/pm_rps/ subcas | 2015-03-23 | Yes|
| 89872 | [ HSW Bisected ] VGA was white screen when re | 2015-04-02 | Yes|
| 90065 | [BSW Regression]igt/gem_userptr_blits/minor-n | 2015-04-17 | No |
| 90112 | [BSW bisected] OglGSCloth/Lightsmark/CS/ Port | 2015-04-20 | Yes|
| 90134 | [BSW Bisected]GFXBench3_gl_driver/GFXBench3_g | 2015-04-22 | Yes|
| 90212 | [BDW-Y bisected] Screen flicker when start X  | 2015-04-28 | Yes|
| 90368 | [SNB bisected igt/kms_3d has hardcoded expect | 2015-05-08 | Yes|
| 90394 | [SNB Regression]igt/kms_plane/plane-position- | 2015-05-11 | No |
| 90461 | [SKL Regression]boot system causes WARNING: C | 2015-05-15 | No |
| 90546 | [BDW/BSW/SKL bisected]igt/pm_rpm/drm-resource | 2015-05-21 | Yes|
| 90732 | [BDW/BSW Bisected]igt/gem_reloc_vs_gpu/forked | 2015-05-29 | Yes|
| 90808 | [BDW Bisected]igt/gem_ctx_param_basic/invalid | 2015-06-02 | Yes|
| 90994 | [BDW regression] pm_rpm subtests fail and giv | 2015-06-16 | No |
| 91378 | [hsw dp regression] 06ea66b6 (5.4GHz link clo | 2015-07-17 | No |
| 91592 | [pnv regression] OOPS on boot | 2015-08-09 | No |
| 91844 | [HSW Regression] intel_do_flush_locked failed | 2015-09-02 | No |
| 91952 | [Bisected Regression] Blank screen from boot  | 2015-09-10 | Yes|
| 91959 | [865g 3.19 regression] Desktop image is disto | 2015-09-10 | No |
| 91974 | [bisected] unrecoverable black screen after k | 2015-09-11 | Yes|
| 92050 | [regression]/bug introduced by commit [0e572f | 2015-09-19 | No |
| 92083 | [regression] [git pull] drm for 4.3   | 2015-09-23 | No |
| 92096 | regression/bug introduced by commit [0e572fe7 | 2015-09-24 | No |
| 92097 | [regression] Linux 4.3-rc  i915: WARNING: int | 2015-09-24 | No |
| 92174 | PROBLEM: Intel VGA output busticated on 4.3-r | 2015-09-29 | No |
| 92211 | [BDW HSW-U SKL Regression] [IGT Basic] igt/pm | 2015-10-01 | No |
| 92237 | Horrible noise (audio) via DisplayPort [regre | 2015-10-02 | No |
| 92324 | [Intel-gfx] [PATCH] fixup! drm/i915/skl: Elim | 2015-10-07 | Yes|
| 92335 | [HSW Regression] Null pointer deference in in | 2015-10-07 | No |
+---+---+++
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/fb-helper: Fix fb refcounting in pan_display_atomic

[Daniel Vetter]

On Fri, Oct 16, 2015 at 07:48:37PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 16, 2015 at 06:23:14PM +0200, Daniel Vetter wrote:
> > In
> > 
> > commit bbb1e52402b2a288b09ae37e8182599931c7e9df
> > Author: Rob Clark 
> > Date:   Tue Aug 25 15:35:58 2015 -0400
> > 
> > drm/fb-helper: atomic restore_fbdev_mode()..
> > 
> > we've forgotten to do the plane->old_fb refcount dance for
> > pan_display_atomic, which can result in refcount leaks if the current
> > configuration is not from fbcon. Which apparently can happen when
> > vt-switching - fbcon does a pan first before a set_par.
> > 
> > OCD-align function parameters while at it.
> > 
> > Cc: Rob Clark 
> > Cc: Rodrigo Vivi 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92483
> > Signed-off-by: Daniel Vetter 
> > ---
> >  drivers/gpu/drm/drm_fb_helper.c | 27 +--
> >  1 file changed, 25 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c 
> > b/drivers/gpu/drm/drm_fb_helper.c
> > index 0ac0fcc9b0d2..b2cf28dd90fe 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -1249,6 +1249,8 @@ retry:
> >  
> > mode_set = _helper->crtc_info[i].mode_set;
> >  
> > +   mode_set->crtc->primary->old_fb = mode_set->crtc->primary->fb;
> > +
> > mode_set->x = var->xoffset;
> > mode_set->y = var->yoffset;
> >  
> > @@ -1264,13 +1266,34 @@ retry:
> > info->var.xoffset = var->xoffset;
> > info->var.yoffset = var->yoffset;
> >  
> > -   return 0;
> >  
> >  fail:
> 
> Time to rename the label too?

Other atomic functions use fail: too. I guess we could do a large-scale
rename to out: or something ...
-Daniel

> 
> > +   for(i = 0; i < fb_helper->crtc_count; i++) {
> > +   struct drm_mode_set *mode_set;
> > +   struct drm_plane *plane;
> > +
> > +   mode_set = _helper->crtc_info[i].mode_set;
> > +   plane = mode_set->crtc->primary;
> > +
> > +   if (ret == 0) {
> > +   struct drm_framebuffer *new_fb = plane->state->fb;
> > +
> > +   if (new_fb)
> > +   drm_framebuffer_reference(new_fb);
> > +   plane->fb = new_fb;
> > +   plane->crtc = plane->state->crtc;
> > +
> > +   if (plane->old_fb)
> > +   drm_framebuffer_unreference(plane->old_fb);
> > +   }
> > +   plane->old_fb = NULL;
> > +   }
> > +
> > if (ret == -EDEADLK)
> > goto backoff;
> >  
> > -   drm_atomic_state_free(state);
> > +   if (ret != 0)
> > +   drm_atomic_state_free(state);
> >  
> > return ret;
> >  
> > -- 
> > 2.5.1
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [Intel-gfx] [PATCH 2/2] drm/fb-helper: Fix fb refcounting in pan_display_atomic

[Daniel Vetter]

On Fri, Oct 16, 2015 at 04:48:12PM +, Rodrigo Vivi wrote:
> Thanks!
> 
> Tested-by: Rodrigo Vivi 
> 
> 
> On Fri, Oct 16, 2015 at 9:35 AM Rob Clark  wrote:
> 
> > On Fri, Oct 16, 2015 at 12:23 PM, Daniel Vetter 
> > wrote:
> > > In
> > >
> > > commit bbb1e52402b2a288b09ae37e8182599931c7e9df
> > > Author: Rob Clark 
> > > Date:   Tue Aug 25 15:35:58 2015 -0400
> > >
> > > drm/fb-helper: atomic restore_fbdev_mode()..
> > >
> > > we've forgotten to do the plane->old_fb refcount dance for
> > > pan_display_atomic, which can result in refcount leaks if the current
> > > configuration is not from fbcon. Which apparently can happen when
> > > vt-switching - fbcon does a pan first before a set_par.
> >
> > oh, whoops
> >
> > > OCD-align function parameters while at it.
> >
> > did you mean to drop that line from the commit msg, or include an
> > extra hunk that you missed?

git add fail, so fixed that and applied the patches, thanks for the review
and testing.
-Daniel

> >
> > at any rate,
> >
> > Reviewed-by: Rob Clark 
> >
> > > Cc: Rob Clark 
> > > Cc: Rodrigo Vivi 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92483
> > > Signed-off-by: Daniel Vetter 
> > > ---
> > >  drivers/gpu/drm/drm_fb_helper.c | 27 +--
> > >  1 file changed, 25 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > b/drivers/gpu/drm/drm_fb_helper.c
> > > index 0ac0fcc9b0d2..b2cf28dd90fe 100644
> > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > @@ -1249,6 +1249,8 @@ retry:
> > >
> > > mode_set = _helper->crtc_info[i].mode_set;
> > >
> > > +   mode_set->crtc->primary->old_fb =
> > mode_set->crtc->primary->fb;
> > > +
> > > mode_set->x = var->xoffset;
> > > mode_set->y = var->yoffset;
> > >
> > > @@ -1264,13 +1266,34 @@ retry:
> > > info->var.xoffset = var->xoffset;
> > > info->var.yoffset = var->yoffset;
> > >
> > > -   return 0;
> > >
> > >  fail:
> > > +   for(i = 0; i < fb_helper->crtc_count; i++) {
> > > +   struct drm_mode_set *mode_set;
> > > +   struct drm_plane *plane;
> > > +
> > > +   mode_set = _helper->crtc_info[i].mode_set;
> > > +   plane = mode_set->crtc->primary;
> > > +
> > > +   if (ret == 0) {
> > > +   struct drm_framebuffer *new_fb =
> > plane->state->fb;
> > > +
> > > +   if (new_fb)
> > > +   drm_framebuffer_reference(new_fb);
> > > +   plane->fb = new_fb;
> > > +   plane->crtc = plane->state->crtc;
> > > +
> > > +   if (plane->old_fb)
> > > +
> >  drm_framebuffer_unreference(plane->old_fb);
> > > +   }
> > > +   plane->old_fb = NULL;
> > > +   }
> > > +
> > > if (ret == -EDEADLK)
> > > goto backoff;
> > >
> > > -   drm_atomic_state_free(state);
> > > +   if (ret != 0)
> > > +   drm_atomic_state_free(state);
> > >
> > > return ret;
> > >
> > > --
> > > 2.5.1
> > >
> >

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

Re: [Intel-gfx] [PATCH 5/5] drm: Check plane src coordinates correctly during page flip for atomic drivers

[Matt Roper]

On Fri, Oct 16, 2015 at 07:17:31PM +0200, Daniel Vetter wrote:
> On Fri, Oct 16, 2015 at 05:40:59PM +0100, Tvrtko Ursulin wrote:
> > 
> > On 16/10/15 17:27, Matt Roper wrote:
> > >On Thu, Oct 15, 2015 at 08:40:02PM +0300, ville.syrj...@linux.intel.com 
> > >wrote:
> > >>From: Ville Syrjälä 
> > >>
> > >>Instead of relying on the old crtc-{x,y,mode} gunk, dig out the primary
> > >>plane coordinates from the plane state when checking them against the
> > >>new framebuffer during page flip.
> > >>
> > >>Cc: Matt Roper 
> > >>Cc: Tvrtko Ursulin 
> > >>Cc: Daniel Vetter 
> > >>Signed-off-by: Ville Syrjälä 
> > >
> > >For the series:
> > >
> > >Reviewed-by: Matt Roper 
> 
> Pulled all 5 into drm-misc, thanks.
> 
> > >I also confirmed that the i-g-t test I wrote here:
> > >
> > > http://lists.freedesktop.org/archives/intel-gfx/2015-October/077394.html
> > >now passes with your patch series, so I believe Tvrtko's original bug
> > >report should be fixed.
> > 
> > Oh I did not realize this series is about this, perhaps because I did not
> > see 1/5 which maybe had some more obvious clues. :)
> > 
> > Great, that means if we decide to merge "drm/i915: Consider plane rotation
> > when calculating stride in skl_do_mmio_flip" and "kms_rotation_crc: Exercise
> > page flips with 90 degree rotation" we would have working rotated legacy
> > page flip with a test case.
> 
> Matt, can you please push these igt patches and double-check that we have
> subtests to catch the viewport confusion here?
> 
> Thanks, Daniel

Looks like Thomas already beat me to pushing the relevant i-g-t patches
(both the one I wrote and the one Tvrtko wrote), so I think we're
covered there.

Thanks.


Matt

> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

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

[Intel-gfx] [PATCH] drm: Explicitly compute the last cacheline for clflush on range

[Chris Wilson]

Fixes regression from

commit afcd950cafea6e27b739fe7772cbbeed37d05b8b
Author: Chris Wilson 
Date:   Wed Jun 10 15:58:01 2015 +0100

drm: Avoid the double clflush on the last cache line in 
drm_clflush_virt_range()

I'm stumped. Looking at the loop we should be iterating over every cache
line until we reach the start of the cacheline after the end of the
virtual range. Evidence says otherwise.

More bizarely, I stored the last address to be clflushed and found it to
be equal to the start of the cacheline containing the last byte. Doubly
purplexed.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92501
Testcase: gem_tiled_partial_pwrite_pread/reads
Signed-off-by: Chris Wilson 
Cc: Imre Deak 
Cc: Daniel Vetter 
---
 drivers/gpu/drm/drm_cache.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
index 6743ff7dccfa..7c909bc8b68a 100644
--- a/drivers/gpu/drm/drm_cache.c
+++ b/drivers/gpu/drm/drm_cache.c
@@ -131,10 +131,13 @@ drm_clflush_virt_range(void *addr, unsigned long length)
 #if defined(CONFIG_X86)
if (cpu_has_clflush) {
const int size = boot_cpu_data.x86_clflush_size;
-   void *end = addr + length;
-   addr = (void *)(((unsigned long)addr) & -size);
+   void *end;
+
+   end = (void *)(((unsigned long)addr + length - 1) & -size);
+   addr = (void *)((unsigned long)addr & -size);
+
mb();
-   for (; addr < end; addr += size)
+   for (; addr <= end; addr += size)
clflushopt(addr);
mb();
return;
-- 
2.6.1

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

Re: [Intel-gfx] [RFC 0/6] Non perf based Gen Graphics OA unit driver

[Peter Zijlstra]

On Fri, Oct 16, 2015 at 12:02:28PM +0200, Ingo Molnar wrote:
> 
> * Peter Zijlstra  wrote:
> 
> > > - We may be making some technical compromises a.t.m for the sake of
> > >   using perf.
> > > 
> > > perf_event_open() requires events to either relate to a pid or a
> > > specific cpu core, while our device pmu relates to neither.  Events
> > > opened with a pid will be automatically enabled/disabled according
> > > to the scheduling of that process - so not appropriate for us.
> > 
> > Right; the traditional cpu/pid mapping doesn't work well for devices;
> > but maybe, with some work, we can create something like that
> > global/local render context from it; although I've no clue what form
> > that would need at this time.
> 
> Could someone please help with some very basic questions, such as what the 
> hardware model of the 'OA' unit model is? How are OA registers set up, how 
> are 
> their values made accessible to the host side, etc.

Robert linked to:

  
https://01.org/sites/default/files/documentation/observability_performance_counters_haswell.pdf

In a previous posting. It has some info, but full documentation, is as
per the initial post, 'pending'.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v3] drm/i915: Improve kernel-doc for i915_audio_component struct

[David Henningsson]

Signed-off-by: David Henningsson 
---

Now rebased against drm-intel git master.

 include/drm/i915_component.h | 69 
 1 file changed, 51 insertions(+), 18 deletions(-)

diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
index 30d89e0..fab1385 100644
--- a/include/drm/i915_component.h
+++ b/include/drm/i915_component.h
@@ -31,47 +31,80 @@
 #define MAX_PORTS 5
 
 /**
- * struct i915_audio_component_ops - callbacks defined in gfx driver
- * @owner: the module owner
- * @get_power: get the POWER_DOMAIN_AUDIO power well
- * @put_power: put the POWER_DOMAIN_AUDIO power well
- * @codec_wake_override: Enable/Disable generating the codec wake signal
- * @get_cdclk_freq: get the Core Display Clock in KHz
- * @sync_audio_rate: set n/cts based on the sample rate
+ * struct i915_audio_component_ops - Ops implemented by i915 driver, called by 
hda driver
  */
 struct i915_audio_component_ops {
+   /**
+* @owner: i915 module
+*/
struct module *owner;
+   /**
+* @get_power: get the POWER_DOMAIN_AUDIO power well
+*
+* Request the power well to be turned on.
+*/
void (*get_power)(struct device *);
+   /**
+* @put_power: put the POWER_DOMAIN_AUDIO power well
+*
+* Allow the power well to be turned off.
+*/
void (*put_power)(struct device *);
+   /**
+* @codec_wake_override: Enable/disable codec wake signal
+*/
void (*codec_wake_override)(struct device *, bool enable);
+   /**
+* @get_cdclk_freq: Get the Core Display Clock in kHz
+*/
int (*get_cdclk_freq)(struct device *);
+   /**
+* @sync_audio_rate: set n/cts based on the sample rate
+*
+* Called from audio driver. After audio driver sets the
+* sample rate, it will call this function to set n/cts
+*/
int (*sync_audio_rate)(struct device *, int port, int rate);
 };
 
+/**
+ * struct i915_audio_component_audio_ops - Ops implemented by hda driver, 
called by i915 driver
+ */
 struct i915_audio_component_audio_ops {
+   /**
+* @audio_ptr: Pointer to be used in call to pin_eld_notify
+*/
void *audio_ptr;
/**
-* Call from i915 driver, notifying the HDA driver that
-* pin sense and/or ELD information has changed.
-* @audio_ptr:  HDA driver object
-* @port:   Which port has changed (PORTA / PORTB / PORTC etc)
+* @pin_eld_notify: Notify the HDA driver that pin sense and/or ELD 
information has changed
+*
+* Called when the i915 driver has set up audio pipeline or has just
+* begun to tear it down. This allows the HDA driver to update its
+* status accordingly (even when the HDA controller is in power save
+* mode).
 */
void (*pin_eld_notify)(void *audio_ptr, int port);
 };
 
 /**
- * struct i915_audio_component - used for audio video interaction
- * @dev: the device from gfx driver
- * @aud_sample_rate: the array of audio sample rate per port
- * @ops: callback for audio driver calling
- * @audio_ops: Call from i915 driver
+ * struct i915_audio_component - Used for direct communication between i915 
and hda drivers
  */
 struct i915_audio_component {
+   /**
+* @dev: i915 device, used as parameter for ops
+*/
struct device *dev;
+   /**
+* @aud_sample_rate: the array of audio sample rate per port
+*/
int aud_sample_rate[MAX_PORTS];
-
+   /**
+* @ops: Ops implemented by i915 driver, called by hda driver
+*/
const struct i915_audio_component_ops *ops;
-
+   /**
+* @audio_ops: Ops implemented by hda driver, called by i915 driver
+*/
const struct i915_audio_component_audio_ops *audio_ops;
 };
 
-- 
1.9.1

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

[Intel-gfx] [PATCH v6] drm/i915: Add soft-pinning API for execbuffer

[Thomas Daniel]

From: Chris Wilson 

Userspace can pass in an offset that it presumes the object is located
at. The kernel will then do its utmost to fit the object into that
location. The assumption is that userspace is handling its own object
locations (for example along with full-ppgtt) and that the kernel will
rarely have to make space for the user's requests.

Signed-off-by: Chris Wilson 

v2: Fixed incorrect eviction found by Michal Winiarski - fix suggested by Chris
Wilson.  Fixed incorrect error paths causing crash found by Michal Winiarski.
(Not published externally)

v3: Rebased because of trivial conflict in object_bind_to_vm.  Fixed eviction
to allow eviction of soft-pinned objects when another soft-pinned object used
by a subsequent execbuffer overlaps reported by Michal Winiarski.
(Not published externally)

v4: Moved soft-pinned objects to the front of ordered_vmas so that they are
pinned first after an address conflict happens to avoid repeated conflicts in
rare cases (Suggested by Chris Wilson).  Expanded comment on
drm_i915_gem_exec_object2.offset to cover this new API.

v5: Added I915_PARAM_HAS_EXEC_SOFTPIN parameter for detecting this capability
(Kristian). Added check for multiple pinnings on eviction (Akash). Made sure
buffers are not considered misplaced without the user specifying
EXEC_OBJECT_SUPPORTS_48B_ADDRESS.  User must assume responsibility for any
addressing workarounds.  Updated object2.offset field comment again to clarify
NO_RELOC case (Chris).  checkpatch cleanup.

v6: Trivial rebase on latest drm-intel-nightly

Cc: Chris Wilson 
Cc: Akash Goel 
Cc: Vinay Belgaumkar 
Cc: Michal Winiarski 
Cc: Zou Nanhai 
Cc: Kristian Høgsberg 
Signed-off-by: Thomas Daniel 
---
 drivers/gpu/drm/i915/i915_dma.c|  3 ++
 drivers/gpu/drm/i915/i915_drv.h|  2 +
 drivers/gpu/drm/i915/i915_gem.c| 64 --
 drivers/gpu/drm/i915/i915_gem_evict.c  | 39 ++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 19 +++--
 include/uapi/drm/i915_drm.h| 12 --
 6 files changed, 113 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2336af9..824c6c3 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -170,6 +170,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
case I915_PARAM_HAS_RESOURCE_STREAMER:
value = HAS_RESOURCE_STREAMER(dev);
break;
+   case I915_PARAM_HAS_EXEC_SOFTPIN:
+   value = 1;
+   break;
default:
DRM_DEBUG("Unknown parameter %d\n", param->param);
return -EINVAL;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1396af9..73c3acf 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2816,6 +2816,7 @@ void i915_gem_vma_destroy(struct i915_vma *vma);
 #define PIN_UPDATE (1<<5)
 #define PIN_ZONE_4G(1<<6)
 #define PIN_HIGH   (1<<7)
+#define PIN_OFFSET_FIXED   (1<<8)
 #define PIN_OFFSET_MASK (~4095)
 int __must_check
 i915_gem_object_pin(struct drm_i915_gem_object *obj,
@@ -3163,6 +3164,7 @@ int __must_check i915_gem_evict_something(struct 
drm_device *dev,
  unsigned long start,
  unsigned long end,
  unsigned flags);
+int __must_check i915_gem_evict_for_vma(struct i915_vma *target);
 int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle);
 
 /* belongs in i915_gem_gtt.h */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1e67484..c3453bd 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3450,30 +3450,50 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object 
*obj,
if (IS_ERR(vma))
goto err_unpin;
 
-   if (flags & PIN_HIGH) {
-   search_flag = DRM_MM_SEARCH_BELOW;
-   alloc_flag = DRM_MM_CREATE_TOP;
+   if (flags & PIN_OFFSET_FIXED) {
+   uint64_t offset = flags & PIN_OFFSET_MASK;
+
+   if (offset & (alignment - 1)) {
+   ret = -EINVAL;
+   goto err_free_vma;
+   }
+   vma->node.start = offset;
+   vma->node.size = size;
+   vma->node.color = obj->cache_level;
+   ret = drm_mm_reserve_node(>mm, >node);
+   if (ret) {
+   ret = i915_gem_evict_for_vma(vma);
+   if (ret == 0)
+   ret = drm_mm_reserve_node(>mm, >node);
+   }
+

[Intel-gfx] [PATCH i-g-t 3/3] kms_rotation_crc: Test case for rotated VMA first with legacy tiling

[Tvrtko Ursulin]

From: Tvrtko Ursulin 

Specifically targetting a WARN_ON in the kernel, i915_gem_fence.c:

if (WARN_ON(!obj->map_and_fenceable))
return -EINVAL;

Which happens for objects with legacy tiling set to Y at the point
object is pinned to display with no normal VMA in existance and
hence obj->map_and_fenceable false.

Signed-off-by: Tvrtko Ursulin 
Cc: Vivek Kasireddy 
---
 tests/kms_rotation_crc.c | 121 +--
 1 file changed, 96 insertions(+), 25 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index cc9847ecc113..6b16ec73bb9a 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -81,7 +81,8 @@ paint_squares(data_t *data, drmModeModeInfo *mode, 
igt_rotation_t rotation,
cairo_destroy(cr);
 }
 
-static void commit_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
+static void commit_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane,
+   bool render)
 {
igt_display_t *display = >display;
enum igt_commit_style commit = COMMIT_LEGACY;
@@ -108,17 +109,18 @@ static void commit_crtc(data_t *data, igt_output_t 
*output, igt_plane_t *plane)
commit = COMMIT_UNIVERSAL;
}
 
-   igt_display_commit2(display, commit);
+   if (render)
+   igt_display_commit2(display, commit);
 }
 
 static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
-igt_plane_t *plane)
+igt_plane_t *plane, bool render)
 {
drmModeModeInfo *mode;
int fb_id, fb_modeset_id;
unsigned int w, h;
-   uint64_t tiling = data->override_tiling ?
- data->override_tiling : LOCAL_DRM_FORMAT_MOD_NONE;
+   uint64_t tiling;
+   unsigned int obj_tiling = !render ? I915_TILING_Y : I915_TILING_NONE;
uint32_t pixel_format = data->override_fmt ?
data->override_fmt : DRM_FORMAT_XRGB;
 
@@ -133,6 +135,14 @@ static void prepare_crtc(data_t *data, igt_output_t 
*output, enum pipe pipe,
w = mode->hdisplay;
h = mode->vdisplay;
 
+   if (data->override_tiling)
+   tiling = data->override_tiling;
+   else if (data->rotation == IGT_ROTATION_90 ||
+data->rotation == IGT_ROTATION_270)
+   tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
+   else
+   tiling = LOCAL_DRM_FORMAT_MOD_NONE;
+
fb_modeset_id = igt_create_fb(data->gfx_fd,
  w, h,
  pixel_format,
@@ -146,8 +156,6 @@ static void prepare_crtc(data_t *data, igt_output_t 
*output, enum pipe pipe,
 */
if (data->rotation == IGT_ROTATION_90 ||
data->rotation == IGT_ROTATION_270) {
-   tiling = data->override_tiling ?
-data->override_tiling : LOCAL_I915_FORMAT_MOD_Y_TILED;
w = h =  mode->vdisplay;
} else if (plane->is_cursor) {
pixel_format = data->override_fmt ?
@@ -158,24 +166,29 @@ static void prepare_crtc(data_t *data, igt_output_t 
*output, enum pipe pipe,
data->w = w;
data->h = h;
 
-   fb_id = igt_create_fb(data->gfx_fd,
- w, h,
- pixel_format,
- tiling,
- >fb);
+   fb_id = igt_create_fb_with_bo_size(data->gfx_fd,
+  w, h,
+  pixel_format,
+  tiling, obj_tiling,
+  >fb, 0);
igt_assert(fb_id);
 
-   /* Step 1: create a reference CRC for a software-rotated fb */
-   paint_squares(data, mode, data->rotation, plane);
-   commit_crtc(data, output, plane);
-   igt_pipe_crc_collect_crc(data->pipe_crc, >ref_crc);
-
-   /*
-* Step 2: prepare the plane with an non-rotated fb let the hw
-* rotate it.
-*/
-   paint_squares(data, mode, IGT_ROTATION_0, plane);
-   igt_plane_set_fb(plane, >fb);
+   if (render) {
+   /* Step 1: create a reference CRC for a software-rotated fb */
+   paint_squares(data, mode, data->rotation, plane);
+   commit_crtc(data, output, plane, render);
+   igt_pipe_crc_collect_crc(data->pipe_crc, >ref_crc);
+
+   /*
+   * Step 2: prepare the plane with an non-rotated fb let the hw
+   * rotate it.
+   */
+   paint_squares(data, mode, IGT_ROTATION_0, plane);
+   igt_plane_set_fb(plane, >fb);
+   } else {
+   commit_crtc(data, output, plane, render);
+   igt_plane_set_fb(plane, >fb);
+   }
 

[Intel-gfx] [PATCH i-g-t 2/3] lib/igt_kms: Set new rotation property before displaying

[Tvrtko Ursulin]

From: Tvrtko Ursulin 

This is not really tested that much apart that it doesn't break
kms_rotation_crc and it makes one new test case work.

It only serves as proof of concept to demonstrate a particular bug.

Signed-off-by: Tvrtko Ursulin 
Cc: Vivek Kasireddy 
Cc: Damien Lespiau 
---
 lib/igt_kms.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 51d735d29970..5c9b358ae23f 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1316,6 +1316,14 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
igt_assert(igt_plane_supports_rotation(plane) ||
   !plane->rotation_changed);
 
+   if (plane->rotation_changed) {
+   ret = igt_plane_set_property(plane, plane->rotation_property,
+  plane->rotation);
+
+   plane->rotation_changed = false;
+   CHECK_RETURN(ret, fail_on_error);
+   }
+
fb_id = igt_plane_get_fb_id(plane);
crtc_id = output->config.crtc->crtc_id;
 
@@ -1377,14 +1385,6 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
plane->position_changed = false;
plane->size_changed = false;
 
-   if (plane->rotation_changed) {
-   ret = igt_plane_set_property(plane, plane->rotation_property,
-  plane->rotation);
-
-   plane->rotation_changed = false;
-   CHECK_RETURN(ret, fail_on_error);
-   }
-
return 0;
 }
 
-- 
1.9.1

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

[Intel-gfx] [PATCH i-g-t 1/3] lib/igt_fb: Allow specifiying object tiling when creating frame buffers

[Tvrtko Ursulin]

From: Tvrtko Ursulin 

Currently object tiling is inferred from the frame buffer modifier
and only for legacy X scanout.

It is useful to support overriding this selection for certain tests
so add the capability.

Signed-off-by: Tvrtko Ursulin 
Cc: Vivek Kasireddy 
---
 lib/igt_fb.c | 21 ++---
 lib/igt_fb.h |  3 ++-
 tests/kms_flip.c |  3 ++-
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index d04f02c0d035..73be93cb6e63 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -75,7 +75,8 @@ static struct format_desc_struct {
 
 /* helpers to create nice-looking framebuffers */
 static int create_bo_for_fb(int fd, int width, int height, int bpp,
-   uint64_t tiling, unsigned bo_size,
+   uint64_t tiling, unsigned int obj_tiling,
+   unsigned bo_size,
uint32_t *gem_handle_ret,
unsigned *size_ret,
unsigned *stride_ret)
@@ -113,7 +114,9 @@ static int create_bo_for_fb(int fd, int width, int height, 
int bpp,
gem_handle = gem_create(fd, bo_size);
 
if (tiling == LOCAL_I915_FORMAT_MOD_X_TILED)
-   ret = __gem_set_tiling(fd, gem_handle, I915_TILING_X, stride);
+   obj_tiling = I915_TILING_X;
+   if (obj_tiling != I915_TILING_NONE)
+   ret = __gem_set_tiling(fd, gem_handle, obj_tiling, stride);
 
*stride_ret = stride;
*size_ret = size;
@@ -414,7 +417,8 @@ void igt_paint_image(cairo_t *cr, const char *filename,
  */
 unsigned int
 igt_create_fb_with_bo_size(int fd, int width, int height,
-  uint32_t format, uint64_t tiling,
+  uint32_t format,
+  uint64_t tiling, unsigned int obj_tiling,
   struct igt_fb *fb, unsigned bo_size)
 {
uint32_t fb_id;
@@ -426,8 +430,9 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
 
igt_debug("%s(width=%d, height=%d, format=0x%x [bpp=%d], 
tiling=0x%"PRIx64", size=%d)\n",
  __func__, width, height, format, bpp, tiling, bo_size);
-   do_or_die(create_bo_for_fb(fd, width, height, bpp, tiling, bo_size,
-  >gem_handle, >size, >stride));
+   do_or_die(create_bo_for_fb(fd, width, height, bpp, tiling, obj_tiling,
+  bo_size, >gem_handle, >size,
+  >stride));
 
igt_debug("%s(handle=%d, pitch=%d)\n",
  __func__, fb->gem_handle, fb->stride);
@@ -485,7 +490,8 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
 unsigned int igt_create_fb(int fd, int width, int height, uint32_t format,
   uint64_t tiling, struct igt_fb *fb)
 {
-   return igt_create_fb_with_bo_size(fd, width, height, format, tiling, 
fb, 0);
+   return igt_create_fb_with_bo_size(fd, width, height, format,
+ tiling, I915_TILING_NONE, fb, 0);
 }
 
 /**
@@ -714,7 +720,8 @@ static void create_cairo_surface__blit(int fd, struct 
igt_fb *fb)
 */
bpp = igt_drm_format_to_bpp(fb->drm_format);
ret = create_bo_for_fb(fd, fb->width, fb->height, bpp,
-   LOCAL_DRM_FORMAT_MOD_NONE, 0,
+   LOCAL_DRM_FORMAT_MOD_NONE, I915_TILING_NONE,
+   0,
>linear.handle,
>linear.size,
>linear.stride);
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index a07acd2444b8..fa92fd4efd5b 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -71,7 +71,8 @@ enum igt_text_align {
 
 unsigned int
 igt_create_fb_with_bo_size(int fd, int width, int height,
-  uint32_t format, uint64_t tiling,
+  uint32_t format,
+  uint64_t tiling, unsigned int obj_tiling,
   struct igt_fb *fb, unsigned bo_size);
 unsigned int igt_create_fb(int fd, int width, int height, uint32_t format,
   uint64_t tiling, struct igt_fb *fb);
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index a139d402608e..60577d23b9a4 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -1366,7 +1366,8 @@ static void run_test_on_crtc_set(struct test_output *o, 
int *crtc_idxs,
 tiling, >fb_info[0]);
o->fb_ids[1] = igt_create_fb_with_bo_size(drm_fd, o->fb_width, 
o->fb_height,
 igt_bpp_depth_to_drm_format(o->bpp, 
o->depth),
-tiling, >fb_info[1], bo_size);
+tiling, I915_TILING_NONE,
+>fb_info[1], 

[Intel-gfx] [PATCH i-g-t] tests/gem_pwrite_snooped: Fix #pragma GCC weirdness

[ville . syrjala]

From: Ville Syrjälä 

gem_pwrite_snooped was broken when the #pragma GCC stuff was added in
b04691b tests/gem_pwrite_snooped: disable const cast warning

Apparently gcc treats '#pragma GCC' as a C statement. With the current
code memchr_inv() pretty much disappears entirely because gcc thinks
there's an unconditional return in the loop body. Or at least that's my
assumption. Put braces around the if body to fix it.

This is the original asm:
02f0 :
 2f0:   48 85 f6test   %rsi,%rsi
 2f3:   b8 00 00 00 00  mov$0x0,%eax
 2f8:   48 0f 45 c7 cmovne %rdi,%rax
 2fc:   c3  retq

This is with the fix:
02f0 :
 2f0:   48 85 f6test   %rsi,%rsi
 2f3:   74 25   je 31a 
 2f5:   48 01 feadd%rdi,%rsi
 2f8:   80 3f ffcmpb   $0xff,(%rdi)
 2fb:   48 8d 57 01 lea0x1(%rdi),%rdx
 2ff:   74 11   je 312 
 301:   eb 1a   jmp31d 
 303:   0f 1f 44 00 00  nopl   0x0(%rax,%rax,1)
 308:   48 83 c2 01 add$0x1,%rdx
 30c:   80 7a ff ff cmpb   $0xff,-0x1(%rdx)
 310:   75 0b   jne31d 
 312:   48 39 f2cmp%rsi,%rdx
 315:   48 89 d7mov%rdx,%rdi
 318:   75 ee   jne308 
 31a:   31 c0   xor%eax,%eax
 31c:   c3  retq
 31d:   48 89 f8mov%rdi,%rax
 320:   c3  retq

Cc: Thomas Wood 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92227
Signed-off-by: Ville Syrjälä 
---
 tests/gem_pwrite_snooped.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/gem_pwrite_snooped.c b/tests/gem_pwrite_snooped.c
index 5783e3b..61b5404 100644
--- a/tests/gem_pwrite_snooped.c
+++ b/tests/gem_pwrite_snooped.c
@@ -82,11 +82,12 @@ static void *memchr_inv(const void *s, int c, size_t n)
unsigned char uc = c;
 
while (n--) {
-   if (*us != uc)
+   if (*us != uc) {
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wcast-qual"
return (void *) us;
 #pragma GCC diagnostic pop
+   }
us++;
}
 
-- 
2.4.9

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

Re: [Intel-gfx] [RFC 0/6] Non perf based Gen Graphics OA unit driver

[Peter Zijlstra]

On Tue, Sep 29, 2015 at 03:39:03PM +0100, Robert Bragg wrote:
> - We're bridging two complex architectures
> 
> To review this work I think it will be relevant to have a good
> general familiarity with Gen graphics (e.g. thinking about the OA
> unit's interaction with the command streamer and execlist
> scheduling) as well as our userspace architecture and how we're
> consuming OA data within Mesa to implement the
> INTEL_performance_query extension.
>
> On the flip side here, its necessary to understand the perf
> userspace interface (for most this is hidden by tools so the details
> aren't common knowledge) as well as the internal design, considering
> that the PMU we're looking at seems to break several current design
> assumptions. I can only claim a limited familiarity with perf's
> design, just as a result of this work.

Right; but a little effort and patience on both sides should get us
there I think. At worst we'll both learn something new ;-)

> - The current OA PMU driver breaks some significant design assumptions.
> 
> Existing perf pmus are used for profiling work on a cpu and we're
> introducing the idea of _IS_DEVICE pmus with different security
> implications, the need to fake cpu-related data (such as user/kernel
> registers) to fit with perf's current design, and adding _DEVICE
> records as a way to forward device-specific status records.

There are more devices with counters on than GPUs, so I think it might
make sense to look at extending perf to better deal with this.

> The OA unit writes reports of counters into a circular buffer,
> without involvement from the CPU, making our PMU driver the first of
> a kind.

Agreed, this is somewhat 'odd' from where we are today.

> Perf supports groups of counters and allows those to be read via
> transactions internally but transactions currently seem designed to
> be explicitly initiated from the cpu (say in response to a userspace
> read()) and while we could pull a report out of the OA buffer we
> can't trigger a report from the cpu on demand.
>
> Related to being report based; the OA counters are configured in HW
> as a set while perf generally expects counter configurations to be
> orthogonal. Although counters can be associated with a group leader
> as they are opened, there's no clear precedent for being able to
> provide group-wide configuration attributes and no obvious solution
> as yet that's expected to be acceptable to upstream and meets our
> userspace needs.

I'm not entirely sure what you mean with group-wide configuration
attributes; could you elaborate?

> We currently avoid using perf's grouping feature
> and forward OA reports to userspace via perf's 'raw' sample field.
> This suits our userspace well considering how coupled the counters
> are when dealing with normalizing. It would be inconvenient to split
> counters up into separate events, only to require userspace to
> recombine them. 

So IF you were using a group, a single read from the leader can return
you a vector of all values (PERF_FORMAT_GROUP), this avoids having to
do that recombine.

Another option would be to view the arrival of an OA vector in the
datastream as an 'event' and generate a PERF_RECORD_READ in the perf
buffer (which again can use the GROUP vector format).

> Related to counter orthogonality; we can't time share the OA unit,
> while event scheduling is a central design idea within perf for
> allowing userspace to open + enable more events than can be
> configured in HW at any one time.

So we have other PMUs that cannot do this; Gen OA would not be unique in
this. Intel PT for example only allows a single active event.

That said; earlier today I saw:

  
https://www.youtube.com/watch?v=9J3BQcAeHpI=PLe6I3NKr-I4J2oLGXhGOeBMEjh8h10jT3=7

where exactly this feature was mentioned as not fitting well into the
existing GPU performance interfaces (GL_AMD_performance_monitor /
GL_INTEL_performance_query).

So there is hardware (Nvidia) out there that does support this. Also
mentioned was that this hardware has global and local counters, where
the local ones are specific to a rendering context. That is not unlike
the per-cpu / per-task stuff perf does.

> The OA unit is not designed to
> allow re-configuration while in use. We can't reconfigure the OA
> unit without loosing internal OA unit state which we can't access
> explicitly to save and restore. Reconfiguring the OA unit is also
> relatively slow, involving ~100 register writes. From userspace Mesa
> also depends on a stable OA configuration when emitting
> MI_REPORT_PERF_COUNT commands and importantly the OA unit can't be
> disabled while there are outstanding MI_RPC commands lest we hang
> the command streamer.

Right; see the PERF_PMU_CAP_EXCLUSIVE stuff.

> - We may be making some technical 

Re: [Intel-gfx] [PATCH i-g-t] tests/gem_pwrite_snooped: Fix #pragma GCC weirdness

[Chris Wilson]

On Fri, Oct 16, 2015 at 12:44:05PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> gem_pwrite_snooped was broken when the #pragma GCC stuff was added in
> b04691b tests/gem_pwrite_snooped: disable const cast warning
> 
> Apparently gcc treats '#pragma GCC' as a C statement. With the current
> code memchr_inv() pretty much disappears entirely because gcc thinks
> there's an unconditional return in the loop body. Or at least that's my
> assumption. Put braces around the if body to fix it.
> 
> This is the original asm:
> 02f0 :
>  2f0:   48 85 f6test   %rsi,%rsi
>  2f3:   b8 00 00 00 00  mov$0x0,%eax
>  2f8:   48 0f 45 c7 cmovne %rdi,%rax
>  2fc:   c3  retq
> 
> This is with the fix:
> 02f0 :
>  2f0:   48 85 f6test   %rsi,%rsi
>  2f3:   74 25   je 31a 
>  2f5:   48 01 feadd%rdi,%rsi
>  2f8:   80 3f ffcmpb   $0xff,(%rdi)
>  2fb:   48 8d 57 01 lea0x1(%rdi),%rdx
>  2ff:   74 11   je 312 
>  301:   eb 1a   jmp31d 
>  303:   0f 1f 44 00 00  nopl   0x0(%rax,%rax,1)
>  308:   48 83 c2 01 add$0x1,%rdx
>  30c:   80 7a ff ff cmpb   $0xff,-0x1(%rdx)
>  310:   75 0b   jne31d 
>  312:   48 39 f2cmp%rsi,%rdx
>  315:   48 89 d7mov%rdx,%rdi
>  318:   75 ee   jne308 
>  31a:   31 c0   xor%eax,%eax
>  31c:   c3  retq
>  31d:   48 89 f8mov%rdi,%rax
>  320:   c3  retq
> 
> Cc: Thomas Wood 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92227
> Signed-off-by: Ville Syrjälä 

Bah, hangs head in shame. You'll see why.
-Chris

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

Re: [Intel-gfx] [PATCH 1/3] drm: Track drm_mm nodes with an interval tree

[Daniel, Thomas]

Hi,

> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
> David Herrmann
> Sent: Wednesday, October 7, 2015 11:23 AM
> To: Chris Wilson; Daniel Vetter; Intel Graphics Development; dri-
> de...@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 1/3] drm: Track drm_mm nodes with an interval
> tree
> 
> Hi
> 
> On Tue, Oct 6, 2015 at 1:19 PM, Chris Wilson  wrote:
> > On Tue, Oct 06, 2015 at 01:11:56PM +0200, Daniel Vetter wrote:
> >> On Tue, Oct 06, 2015 at 11:53:09AM +0100, Chris Wilson wrote:
> >> > In addition to the last-in/first-out stack for accessing drm_mm nodes,
> >> > we occasionally and in the future often want to find a drm_mm_node by an
> >> > address. To do so efficiently we need to track the nodes in an interval
> >> > tree - lookups for a particular address will then be O(lg(N)), where N
> >> > is the number of nodes in the range manager as opposed to O(N).
> >> > Insertion however gains an extra O(lg(N)) step for all nodes
> >> > irrespective of whether the interval tree is in use. For future i915
> >> > patches, eliminating the linear walk is a significant improvement.
> >> >
> >> > Signed-off-by: Chris Wilson 
> >> > Cc: dri-de...@lists.freedesktop.org
> >>
> >> For the vma manager David Herrman put the interval tree outside of
> drm_mm.
> >> Whichever way we pick, but I think we should be consistent about this.
> >
> > Given that the basis of this patch is that functionality exposed by
> > drm_mm (i.e. drm_mm_reserve_node) is too slow for our use case (i.e.
> > there is a measurable perf degradation if we switch over from the mru
> > stack to using fixed addresses) it makes sense to improve that
> > functionality. The question is then why the drm_vma_manager didn't use
> > and improve the existing functionality...
> 
> I didn't want to slow down drm_mm operations, so I kept it separate. I
> don't mind if it is merged into drm_mm. It'd be trivial to make the
> vma-manager use it (only on the top-level, though).
> 
> Thanks
> David

Is there a conclusion to this discussion?  I'm under increasing pressure to get 
the i915 soft-pinning merged and Chris's latest version depends on this 
interval tree.

I've been told to post a new rebase of the version which doesn't use the 
interval tree if not.

Cheers,
Thomas.

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

Re: [Intel-gfx] [RFC 0/6] Non perf based Gen Graphics OA unit driver

[Ingo Molnar]

* Peter Zijlstra  wrote:

> > - We may be making some technical compromises a.t.m for the sake of
> >   using perf.
> > 
> > perf_event_open() requires events to either relate to a pid or a
> > specific cpu core, while our device pmu relates to neither.  Events
> > opened with a pid will be automatically enabled/disabled according
> > to the scheduling of that process - so not appropriate for us.
> 
> Right; the traditional cpu/pid mapping doesn't work well for devices;
> but maybe, with some work, we can create something like that
> global/local render context from it; although I've no clue what form
> that would need at this time.

Could someone please help with some very basic questions, such as what the 
hardware model of the 'OA' unit model is? How are OA registers set up, how are 
their values made accessible to the host side, etc.

I see some references to 'OA' registers in:

  
https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-bdw-vol03-gpu_overview_1.pdf

and I tried to find a more high level description in:

   
https://01.org/linuxgraphics/documentation/hardware-specification-prms/2014-2015-intel-processors-based-broadwell-platform

but couldn't find it. (Maybe it's my fault!)

Thanks,

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

Re: [Intel-gfx] [PATCH i-g-t 1/3] lib/igt_fb: Allow specifiying object tiling when creating frame buffers

[Ville Syrjälä]

On Fri, Oct 16, 2015 at 11:59:47AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin 
> 
> Currently object tiling is inferred from the frame buffer modifier
> and only for legacy X scanout.
> 
> It is useful to support overriding this selection for certain tests
> so add the capability.

So you want to set up the object tiling differently from the fb
tiling? Why is that? And don't we reject it in the kernel? If we don't
need a fence for scanout (ie. FBC or gen2/3) we could allow it I
suppose, but not sure it it really helps with anything.

BTW I was thinking of allowing the test to pass in an already created
bo to igt_create_fb to use in some fb->offsets[] tests.

> Signed-off-by: Tvrtko Ursulin 
> Cc: Vivek Kasireddy 
> ---
>  lib/igt_fb.c | 21 ++---
>  lib/igt_fb.h |  3 ++-
>  tests/kms_flip.c |  3 ++-
>  3 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index d04f02c0d035..73be93cb6e63 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -75,7 +75,8 @@ static struct format_desc_struct {
>  
>  /* helpers to create nice-looking framebuffers */
>  static int create_bo_for_fb(int fd, int width, int height, int bpp,
> - uint64_t tiling, unsigned bo_size,
> + uint64_t tiling, unsigned int obj_tiling,
> + unsigned bo_size,
>   uint32_t *gem_handle_ret,
>   unsigned *size_ret,
>   unsigned *stride_ret)
> @@ -113,7 +114,9 @@ static int create_bo_for_fb(int fd, int width, int 
> height, int bpp,
>   gem_handle = gem_create(fd, bo_size);
>  
>   if (tiling == LOCAL_I915_FORMAT_MOD_X_TILED)
> - ret = __gem_set_tiling(fd, gem_handle, I915_TILING_X, stride);
> + obj_tiling = I915_TILING_X;
> + if (obj_tiling != I915_TILING_NONE)
> + ret = __gem_set_tiling(fd, gem_handle, obj_tiling, stride);
>  
>   *stride_ret = stride;
>   *size_ret = size;
> @@ -414,7 +417,8 @@ void igt_paint_image(cairo_t *cr, const char *filename,
>   */
>  unsigned int
>  igt_create_fb_with_bo_size(int fd, int width, int height,
> -uint32_t format, uint64_t tiling,
> +uint32_t format,
> +uint64_t tiling, unsigned int obj_tiling,
>  struct igt_fb *fb, unsigned bo_size)
>  {
>   uint32_t fb_id;
> @@ -426,8 +430,9 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
>  
>   igt_debug("%s(width=%d, height=%d, format=0x%x [bpp=%d], 
> tiling=0x%"PRIx64", size=%d)\n",
> __func__, width, height, format, bpp, tiling, bo_size);
> - do_or_die(create_bo_for_fb(fd, width, height, bpp, tiling, bo_size,
> ->gem_handle, >size, >stride));
> + do_or_die(create_bo_for_fb(fd, width, height, bpp, tiling, obj_tiling,
> +bo_size, >gem_handle, >size,
> +>stride));
>  
>   igt_debug("%s(handle=%d, pitch=%d)\n",
> __func__, fb->gem_handle, fb->stride);
> @@ -485,7 +490,8 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
>  unsigned int igt_create_fb(int fd, int width, int height, uint32_t format,
>  uint64_t tiling, struct igt_fb *fb)
>  {
> - return igt_create_fb_with_bo_size(fd, width, height, format, tiling, 
> fb, 0);
> + return igt_create_fb_with_bo_size(fd, width, height, format,
> +   tiling, I915_TILING_NONE, fb, 0);
>  }
>  
>  /**
> @@ -714,7 +720,8 @@ static void create_cairo_surface__blit(int fd, struct 
> igt_fb *fb)
>*/
>   bpp = igt_drm_format_to_bpp(fb->drm_format);
>   ret = create_bo_for_fb(fd, fb->width, fb->height, bpp,
> - LOCAL_DRM_FORMAT_MOD_NONE, 0,
> + LOCAL_DRM_FORMAT_MOD_NONE, I915_TILING_NONE,
> + 0,
>   >linear.handle,
>   >linear.size,
>   >linear.stride);
> diff --git a/lib/igt_fb.h b/lib/igt_fb.h
> index a07acd2444b8..fa92fd4efd5b 100644
> --- a/lib/igt_fb.h
> +++ b/lib/igt_fb.h
> @@ -71,7 +71,8 @@ enum igt_text_align {
>  
>  unsigned int
>  igt_create_fb_with_bo_size(int fd, int width, int height,
> -uint32_t format, uint64_t tiling,
> +uint32_t format,
> +uint64_t tiling, unsigned int obj_tiling,
>  struct igt_fb *fb, unsigned bo_size);
>  unsigned int igt_create_fb(int fd, int width, int height, uint32_t format,
>  uint64_t tiling, struct igt_fb *fb);
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index a139d402608e..60577d23b9a4 100644
> --- 

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

[Thomas Wood]

On 15 October 2015 at 09:05, Derek Morton  wrote:
> This test is based on gem_exec_nop but submits nop batch buffers concurrently
> from different threads to check for ring hangs and other issues during
> concurrent submissions.

Is there any reason not to include this as extra subtests in
gem_exec_nop so that related tests are grouped together?


>
> Signed-off-by: Derek Morton 
> ---
>  tests/.gitignore|   1 +
>  tests/Makefile.sources  |   1 +
>  tests/gem_exec_nop_concurrent.c | 172 
> 
>  3 files changed, 174 insertions(+)
>  create mode 100644 tests/gem_exec_nop_concurrent.c
>
> diff --git a/tests/.gitignore b/tests/.gitignore
> index dc8bb53..0ad36f3 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -46,6 +46,7 @@ gem_exec_blt
>  gem_exec_faulting_reloc
>  gem_exec_lut_handle
>  gem_exec_nop
> +gem_exec_nop_concurrent
>  gem_exec_params
>  gem_exec_parse
>  gem_fd_exhaustion
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 2e2e088..aece831 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -27,6 +27,7 @@ TESTS_progs_M = \
> gem_exec_bad_domains \
> gem_exec_faulting_reloc \
> gem_exec_nop \
> +   gem_exec_nop_concurrent \
> gem_exec_params \
> gem_exec_parse \
> gem_fenced_exec_thrash \
> diff --git a/tests/gem_exec_nop_concurrent.c b/tests/gem_exec_nop_concurrent.c
> new file mode 100644
> index 000..578f651
> --- /dev/null
> +++ b/tests/gem_exec_nop_concurrent.c
> @@ -0,0 +1,172 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *Derek Morton 
> + *
> + * This test is based on gem_exec_nop but submits nop batch buffers 
> concurrently
> + * from different threads to check for ring hangs and other issues during
> + * concurrent submissions.
> + *
> + */
> +
> +#include "igt.h"
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "drm.h"
> +
> +#define LOCAL_I915_EXEC_NO_RELOC (1<<11)
> +#define LOCAL_I915_EXEC_HANDLE_LUT (1<<12)
> +
> +#define LOCAL_I915_EXEC_VEBOX (4<<0)
> +
> +IGT_TEST_DESCRIPTION(
> +"This Test will submit nop batch buffers concurrently to the same ring "
> + "and different rings in an attempt to trigger ring hangs.");
> +
> +const uint32_t batch[2] = {MI_BATCH_BUFFER_END};
> +
> +struct ring
> +{
> +   unsigned ring_id;
> +   const char *ring_name;
> +   bool direction;
> +};
> +
> +static void loop(int fd, uint32_t handle, int child_nbr, struct ring* ring, 
> bool up)
> +{
> +   struct drm_i915_gem_execbuffer2 execbuf;
> +   struct drm_i915_gem_exec_object2 gem_exec[1];
> +   int count;
> +   int max_count = SLOW_QUICK(15, 4);
> +
> +   gem_require_ring(fd, ring->ring_id);
> +
> +   memset(_exec, 0, sizeof(gem_exec));
> +   gem_exec[0].handle = handle;
> +
> +   memset(, 0, sizeof(execbuf));
> +   execbuf.buffers_ptr = (uintptr_t)gem_exec;
> +   execbuf.buffer_count = 1;
> +   execbuf.flags = ring->ring_id;
> +   execbuf.flags |= LOCAL_I915_EXEC_HANDLE_LUT;
> +   execbuf.flags |= LOCAL_I915_EXEC_NO_RELOC;
> +   if (drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, )) {
> +   execbuf.flags = ring->ring_id;
> +   do_ioctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, );
> +   }
> +   gem_sync(fd, handle);
> +
> +   for (count = 0; count <= max_count; count++) {
> +   const int reps = 7;
> +   int n, nbr_loops;
> +
> +   if (up)
> +   nbr_loops = 1 << count;
> +   else
> +

Re: [Intel-gfx] [PATCH 2/5] drm/i915: Make prepare_plane_fb fully interruptible.

[Ander Conselvan De Oliveira]

On Wed, 2015-09-23 at 13:27 +0200, Maarten Lankhorst wrote:
> Now that we agreed on not preserving framebuffers pinning is finally
> allowed to fail because of signals. Use this to make pinning
> and acquire the mutex in an interruptible way too.
> 
> Unpinning is still uninterruptible, because it happens as a cleanup
> of old state, or undoing pins after one of the pins failed.
> 
> The intel_pin_and_fence_fb_obj in page_flip will also wait interruptibly,
> and can be aborted now.
> 
> Signed-off-by: Maarten Lankhorst 

Reviewed-by: Ander Conselvan de Oliveira 

> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 +--
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index ac97af69be62..25e1eac260fd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2349,11 +2349,10 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
>*/
>   intel_runtime_pm_get(dev_priv);
>  
> - dev_priv->mm.interruptible = false;
>   ret = i915_gem_object_pin_to_display_plane(obj, alignment, pipelined,
>  pipelined_request, );
>   if (ret)
> - goto err_interruptible;
> + goto err_pm;
>  
>   /* Install a fence for tiled scan-out. Pre-i965 always needs a
>* fence, whereas 965+ only requires a fence if using
> @@ -2377,14 +2376,12 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
>  
>   i915_gem_object_pin_fence(obj);
>  
> - dev_priv->mm.interruptible = true;
>   intel_runtime_pm_put(dev_priv);
>   return 0;
>  
>  err_unpin:
>   i915_gem_object_unpin_from_display_plane(obj, );
> -err_interruptible:
> - dev_priv->mm.interruptible = true;
> +err_pm:
>   intel_runtime_pm_put(dev_priv);
>   return ret;
>  }
> @@ -13305,7 +13302,9 @@ intel_prepare_plane_fb(struct drm_plane *plane,
>   if (!obj && !old_obj)
>   return 0;
>  
> - mutex_lock(>struct_mutex);
> + ret = i915_mutex_lock_interruptible(dev);
> + if (ret)
> + return ret;
>  
>   if (!obj) {
>   ret = 0;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 1/3] lib/igt_fb: Allow specifiying object tiling when creating frame buffers

[Tvrtko Ursulin]

Hi,

On 16/10/15 13:03, Ville Syrjälä wrote:

On Fri, Oct 16, 2015 at 11:59:47AM +0100, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

Currently object tiling is inferred from the frame buffer modifier
and only for legacy X scanout.

It is useful to support overriding this selection for certain tests
so add the capability.


So you want to set up the object tiling differently from the fb
tiling? Why is that? And don't we reject it in the kernel? If we don't
need a fence for scanout (ie. FBC or gen2/3) we could allow it I
suppose, but not sure it it really helps with anything.


Hm, yes and no. Only different in a sense that currently igt_fb leaves 
object tiling at linear regardless of the fb modifier tiling. (Apart for 
the legacy X where it requires that they match.)


I needed a way of having Y tiled fb modifier and Y tiled object to hit a 
warning in i915_gem_object_get_fence when only the rotated view exists.


Patch series is probably to invasive anyway, especially I did not spend 
any time evaluating if 2/3 is safe. So hopefully Vivek can refine his 
version of the testcase which would then be completely confined to 
kms_rotation_crc.


Regards,

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915/gen8: Flip the 48b switch

[Michel Thierry]

On 10/1/2015 2:16 PM, Daniel Vetter wrote:

On Wed, Sep 30, 2015 at 03:36:19PM +0100, Michel Thierry wrote:

Use 48b addresses if hw supports it (i915.enable_ppgtt=3).
Update the sanitize_enable_ppgtt for 48 bit PPGTT mode.

Note, aliasing PPGTT remains 32b only.

v2: s/full_64b/full_48b/. (Akash)
v3: Add sanitize_enable_ppgtt changes until here. (Akash)
v4: Update param description (Chris)

Cc: Akash Goel 
Cc: Chris Wilson 
Signed-off-by: Michel Thierry 


Queued for -next, thanks for the patch. Can you please ping someone from
mesa to push the libdrm/mesa patches too?

Cheers, Daniel



Sorry I didn't report this earlier, but I just realized I can't find 
this patch in the tree.


There's still the discussion of where to apply the 32-bit workaround and 
the different behavior seen in mesa (not if it isn't needed)... but the 
code was written to not require that flag in order to start using the 
4-level page translation.


Thanks,

-Michel


---
  drivers/gpu/drm/i915/i915_gem_gtt.c | 7 ++-
  drivers/gpu/drm/i915/i915_params.c  | 2 +-
  2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 47344d0..2d964a7 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -104,9 +104,11 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, 
int enable_ppgtt)
  {
bool has_aliasing_ppgtt;
bool has_full_ppgtt;
+   bool has_full_48bit_ppgtt;

has_aliasing_ppgtt = INTEL_INFO(dev)->gen >= 6;
has_full_ppgtt = INTEL_INFO(dev)->gen >= 7;
+   has_full_48bit_ppgtt = IS_BROADWELL(dev) || INTEL_INFO(dev)->gen >= 9;

if (intel_vgpu_active(dev))
has_full_ppgtt = false; /* emulation is too hard */
@@ -125,6 +127,9 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, 
int enable_ppgtt)
if (enable_ppgtt == 2 && has_full_ppgtt)
return 2;

+   if (enable_ppgtt == 3 && has_full_48bit_ppgtt)
+   return 3;
+
  #ifdef CONFIG_INTEL_IOMMU
/* Disable ppgtt on SNB if VT-d is on. */
if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped) {
@@ -141,7 +146,7 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, 
int enable_ppgtt)
}

if (INTEL_INFO(dev)->gen >= 8 && i915.enable_execlists)
-   return 2;
+   return has_full_48bit_ppgtt ? 3 : 2;
else
return has_aliasing_ppgtt ? 1 : 0;
  }
diff --git a/drivers/gpu/drm/i915/i915_params.c 
b/drivers/gpu/drm/i915/i915_params.c
index ca9b8f6..368df67 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -111,7 +111,7 @@ MODULE_PARM_DESC(enable_hangcheck,
  module_param_named_unsafe(enable_ppgtt, i915.enable_ppgtt, int, 0400);
  MODULE_PARM_DESC(enable_ppgtt,
"Override PPGTT usage. "
-   "(-1=auto [default], 0=disabled, 1=aliasing, 2=full)");
+   "(-1=auto [default], 0=disabled, 1=aliasing, 2=full, 3=full with extended 
address space)");

  module_param_named_unsafe(enable_execlists, i915.enable_execlists, int, 0400);
  MODULE_PARM_DESC(enable_execlists,
--
2.6.0

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



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

Re: [Intel-gfx] [PATCH 1/2] drm/i915/skl: Allow universal planes to position

[Tvrtko Ursulin]

On 08/10/15 09:58, Tvrtko Ursulin wrote:

On 07/10/15 15:19, Daniel Vetter wrote:

On Tue, Oct 06, 2015 at 07:28:10PM +0300, Ville Syrjälä wrote:

On Tue, Oct 06, 2015 at 08:16:19AM -0700, Matt Roper wrote:

On Tue, Oct 06, 2015 at 05:42:42PM +0300, Ville Syrjälä wrote:

On Tue, Oct 06, 2015 at 07:29:54AM -0700, Matt Roper wrote:

On Tue, Oct 06, 2015 at 02:32:47PM +0100, Tvrtko Ursulin wrote:


On 10/04/15 10:07, Sonika Jindal wrote:

Signed-off-by: Sonika Jindal 
Reviewed-by: Matt Roper 
---
  drivers/gpu/drm/i915/intel_display.c |7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index ceb2e61..f0bbc22 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct
drm_plane *plane,
  struct drm_rect *dest = >dst;
  struct drm_rect *src = >src;
  const struct drm_rect *clip = >clip;
+bool can_position = false;
  int ret;

  crtc = crtc ? crtc : plane->crtc;
  intel_crtc = to_intel_crtc(crtc);

+if (INTEL_INFO(dev)->gen >= 9)
+can_position = true;
+
  ret = drm_plane_helper_check_update(plane, crtc, fb,
  src, dest, clip,
  DRM_PLANE_HELPER_NO_SCALING,
  DRM_PLANE_HELPER_NO_SCALING,
-false, true, >visible);
+can_position, true,
+>visible);
  if (ret)
  return ret;




I have discovered today that, while this allows SetCrtc and SetPlane
ioctls to work with frame buffers which do not cover the plane, page
flips are not that lucky and fail roughly with:

[drm:drm_crtc_check_viewport] Invalid fb size 1080x1080 for CRTC
viewport 1920x1080+0+0.


Maybe I'm misunderstanding your explanation, but a framebuffer is
always
required to fill/cover the plane scanning out of it.  What this
patch is
supposed to be allowing is for the primary plane to not cover the
entire
CRTC (since that's something that only became possible for Intel
hardware on the gen9+ platforms).  I.e., the primary plane is now
allowed to positioned and resized to cover a subset of the CRTC area,
just like "sprite" planes have always been able to.

If you've got a 1080x1080 framebuffer, then it's legal to have a
1080x1080 primary plane while running in 1920x1080 mode on SKL/BXT.
However it is not legal to size the primary plane as 1920x1080 and
use
this same 1080x1080 framebuffer with any of our interfaces (setplane,
setcrtc, pageflip, or atomic).

Are you using ioctls/libdrm directly or are you using igt_kms
helpers?
IIRC, the IGT helpers will try to be extra helpful and automatically
size the plane to match the framebuffer (unless you override that
behavior), so that might be what's causing the confusion here.


The problem is clear as day in drm_mode_page_flip_ioctl():
ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y, >mode,
fb);
if (ret)
goto out;

The fix should be easy; just extract the current src coordinates from
the plane state and check those against the new fb size. And then hope
that the plane state is really up to date.


Yep, that's the conclusion we came to once Tvrtko explained what he was
seeing on IRC.  I'm not sure whether non-atomic drivers have enough
state setup by the default helpers to work properly.  Worst case we'll
just assume that a non-atomic driver won't support primary plane
windowing (since none have in the past) and fall back to looking at the
mode for legacy non-atomic drivers.



And I'm sure rotated cases will go boom in some other ways. Probably
we should just switch over to using the full plane update for mmio
flips to fix it.


Yeah; the core looks at a drm_plane->invert_dimensions field that's
only
set by omap.  That should probably be updated to look at the state's
rotation on atomic-capable drivers.


We can just look at the src coordinates. Those always match the fb
orientation.


Can we just not bother with legacy pageflips on rotated planes? setplane
works and once you rotate it kinda gets nasty anyway.


I don't know - thought it is simple enough to make it work so why not?
Just " [PATCH] drm/i915: Consider plane rotation when calculating stride
in skl_do_mmio_flip" I posted, plus Matt's "[PATCH] drm: Check fb
against plane size rather than CRTC mode for pageflip​" to allow smaller
than mode planes.


The problem I see is that with legacy pageflip we also need to hack up
something that doesn't look at plane->state for legacy and all that for a
grand total of about 2 drivers, both getting converted to atomic.


I'll leave the legacy/atomic/etc considerations to the experts. :)


Are we sure any efforts to support rotation in legacy page flips is not 
worth it?


So far there were three patches for this: Plane programming fix (very 
simple) and an IGT test case 

Re: [Intel-gfx] [DMC_BUGFIX_V3] drm/i915/skl: Making DC6 entry is the last call in suspend flow.

[Imre Deak]

On ti, 2015-09-29 at 11:01 +0530, Animesh Manna wrote:
> Mmio register access after dc6/dc5 entry is not allowed when
> DC6 power states are enabled according to bspec (bspec-id 0527),
> so enabling dc6 as the last call in suspend flow.
> 
> v1: Initial version.
> 
> v2: Based on review comment from Daniel,
> - created a seperate patch for csr uninitialization set call.
> 
> v3: Rebased on top of latest code.
> 
> Cc: Daniel Vetter 
> Cc: Damien Lespiau 
> Cc: Imre Deak 
> Cc: Sunil Kamath 
> Signed-off-by: Animesh Manna 
> Signed-off-by: Vathsala Nagaraju 
> Signed-off-by: Rajneesh Bhardwaj 

Acked-by: Imre Deak 

Suggestion for the commit message:

Currently we keep DC6 enabled during modesets and DPAUX transfers, which
is not allowed according to the specification. This can lead at least to
PLL locking failures, DPAUX timeouts and prevent deeper package power
states (PC9/10). Fix this for now by enabling DC6 only when we know the
above events (modeset, DPAUX) can't happen.

This a temporary solution as some issues are still unsolved as described
in [1] and [2], we'll address those as a follow-up. 

[1]
http://lists.freedesktop.org/archives/intel-gfx/2015-October/077669.html
[2]
http://lists.freedesktop.org/archives/intel-gfx/2015-October/077787.html


> ---
>  drivers/gpu/drm/i915/i915_drv.c | 13 +
>  drivers/gpu/drm/i915/intel_drv.h|  2 ++
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 19 +++
>  3 files changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1cb6b82..51075d5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1049,10 +1049,20 @@ static int i915_pm_resume(struct device *dev)
>  
>  static int skl_suspend_complete(struct drm_i915_private *dev_priv)
>  {
> + enum csr_state state;
>   /* Enabling DC6 is not a hard requirement to enter runtime D3 */
>  
>   skl_uninit_cdclk(dev_priv);
>  
> + /* TODO: wait for a completion event or
> +  * similar here instead of busy
> +  * waiting using wait_for function.
> +  */
> + wait_for((state = intel_csr_load_status_get(dev_priv)) !=
> + FW_UNINITIALIZED, 1000);
> + if (state == FW_LOADED)
> + skl_enable_dc6(dev_priv);
> +
>   return 0;
>  }
>  
> @@ -1099,6 +1109,9 @@ static int skl_resume_prepare(struct drm_i915_private 
> *dev_priv)
>  {
>   struct drm_device *dev = dev_priv->dev;
>  
> + if (intel_csr_load_status_get(dev_priv) == FW_LOADED)
> + skl_disable_dc6(dev_priv);
> +
>   skl_init_cdclk(dev_priv);
>   intel_csr_load_program(dev);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index c96289d..990161d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1143,6 +1143,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
>  void bxt_disable_dc9(struct drm_i915_private *dev_priv);
>  void skl_init_cdclk(struct drm_i915_private *dev_priv);
>  void skl_uninit_cdclk(struct drm_i915_private *dev_priv);
> +void skl_enable_dc6(struct drm_i915_private *dev_priv);
> +void skl_disable_dc6(struct drm_i915_private *dev_priv);
>  void intel_dp_get_m_n(struct intel_crtc *crtc,
> struct intel_crtc_state *pipe_config);
>  void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index d8e9416..d6b4f61 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -551,7 +551,7 @@ static void assert_can_disable_dc6(struct 
> drm_i915_private *dev_priv)
> "DC6 already programmed to be disabled.\n");
>  }
>  
> -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
> +void skl_enable_dc6(struct drm_i915_private *dev_priv)
>  {
>   uint32_t val;
>  
> @@ -568,7 +568,7 @@ static void skl_enable_dc6(struct drm_i915_private 
> *dev_priv)
>   POSTING_READ(DC_STATE_EN);
>  }
>  
> -static void skl_disable_dc6(struct drm_i915_private *dev_priv)
> +void skl_disable_dc6(struct drm_i915_private *dev_priv)
>  {
>   uint32_t val;
>  
> @@ -629,10 +629,10 @@ static void skl_set_power_well(struct drm_i915_private 
> *dev_priv,
>   !I915_READ(HSW_PWR_WELL_BIOS),
>   "Invalid for power well status to be enabled, 
> unless done by the BIOS, \
>   when request is to disable!\n");
> - if ((GEN9_ENABLE_DC5(dev) || SKL_ENABLE_DC6(dev)) &&
> - power_well->data == SKL_DISP_PW_2) {
> +  

Re: [Intel-gfx] [PATCH i-g-t 1/3] lib/igt_fb: Allow specifiying object tiling when creating frame buffers

[Ville Syrjälä]

On Fri, Oct 16, 2015 at 01:19:35PM +0100, Tvrtko Ursulin wrote:
> 
> Hi,
> 
> On 16/10/15 13:03, Ville Syrjälä wrote:
> > On Fri, Oct 16, 2015 at 11:59:47AM +0100, Tvrtko Ursulin wrote:
> >> From: Tvrtko Ursulin 
> >>
> >> Currently object tiling is inferred from the frame buffer modifier
> >> and only for legacy X scanout.
> >>
> >> It is useful to support overriding this selection for certain tests
> >> so add the capability.
> >
> > So you want to set up the object tiling differently from the fb
> > tiling? Why is that? And don't we reject it in the kernel? If we don't
> > need a fence for scanout (ie. FBC or gen2/3) we could allow it I
> > suppose, but not sure it it really helps with anything.
> 
> Hm, yes and no. Only different in a sense that currently igt_fb leaves 
> object tiling at linear regardless of the fb modifier tiling. (Apart for 
> the legacy X where it requires that they match.)
> 
> I needed a way of having Y tiled fb modifier and Y tiled object to hit a 
> warning in i915_gem_object_get_fence when only the rotated view exists.

Is there a problem of just doing what X tiled did also for Y tiled?

> 
> Patch series is probably to invasive anyway, especially I did not spend 
> any time evaluating if 2/3 is safe. So hopefully Vivek can refine his 
> version of the testcase which would then be completely confined to 
> kms_rotation_crc.
> 
> Regards,
> 
> Tvrtko

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

Re: [Intel-gfx] [PATCH i-g-t 1/3] lib/igt_fb: Allow specifiying object tiling when creating frame buffers

[Ville Syrjälä]

On Fri, Oct 16, 2015 at 01:54:51PM +0100, Tvrtko Ursulin wrote:
> 
> On 16/10/15 13:29, Ville Syrjälä wrote:
> > On Fri, Oct 16, 2015 at 01:19:35PM +0100, Tvrtko Ursulin wrote:
> >>
> >> Hi,
> >>
> >> On 16/10/15 13:03, Ville Syrjälä wrote:
> >>> On Fri, Oct 16, 2015 at 11:59:47AM +0100, Tvrtko Ursulin wrote:
>  From: Tvrtko Ursulin 
> 
>  Currently object tiling is inferred from the frame buffer modifier
>  and only for legacy X scanout.
> 
>  It is useful to support overriding this selection for certain tests
>  so add the capability.
> >>>
> >>> So you want to set up the object tiling differently from the fb
> >>> tiling? Why is that? And don't we reject it in the kernel? If we don't
> >>> need a fence for scanout (ie. FBC or gen2/3) we could allow it I
> >>> suppose, but not sure it it really helps with anything.
> >>
> >> Hm, yes and no. Only different in a sense that currently igt_fb leaves
> >> object tiling at linear regardless of the fb modifier tiling. (Apart for
> >> the legacy X where it requires that they match.)
> >>
> >> I needed a way of having Y tiled fb modifier and Y tiled object to hit a
> >> warning in i915_gem_object_get_fence when only the rotated view exists.
> >
> > Is there a problem of just doing what X tiled did also for Y tiled?
> 
> What do you mean? In the kernel or igt_fb? If latter then it needs to be 
> able to have object tiling as linear by default since that is how fb 
> modifiers were intended to be used - decoupled from obj tiling. Is that 
> what you meant?

Just setting obj to Y tiled when fb is Y tiled from igt_fb.

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

Re: [Intel-gfx] [PATCH i-g-t 1/3] lib/igt_fb: Allow specifiying object tiling when creating frame buffers

[Tvrtko Ursulin]

On 16/10/15 13:29, Ville Syrjälä wrote:

On Fri, Oct 16, 2015 at 01:19:35PM +0100, Tvrtko Ursulin wrote:


Hi,

On 16/10/15 13:03, Ville Syrjälä wrote:

On Fri, Oct 16, 2015 at 11:59:47AM +0100, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

Currently object tiling is inferred from the frame buffer modifier
and only for legacy X scanout.

It is useful to support overriding this selection for certain tests
so add the capability.


So you want to set up the object tiling differently from the fb
tiling? Why is that? And don't we reject it in the kernel? If we don't
need a fence for scanout (ie. FBC or gen2/3) we could allow it I
suppose, but not sure it it really helps with anything.


Hm, yes and no. Only different in a sense that currently igt_fb leaves
object tiling at linear regardless of the fb modifier tiling. (Apart for
the legacy X where it requires that they match.)

I needed a way of having Y tiled fb modifier and Y tiled object to hit a
warning in i915_gem_object_get_fence when only the rotated view exists.


Is there a problem of just doing what X tiled did also for Y tiled?


What do you mean? In the kernel or igt_fb? If latter then it needs to be 
able to have object tiling as linear by default since that is how fb 
modifiers were intended to be used - decoupled from obj tiling. Is that 
what you meant?


Regards,

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

Re: [Intel-gfx] [PATCH] igt/kms_addfb_basic: New subtest to check for fb modifier and tiling mode mismatch

[Tvrtko Ursulin]

Hi,

On 16/10/15 02:10, Vivek Kasireddy wrote:

On 07/10/15 03:35, Vivek Kasireddy wrote:

This new subtest will validate a Y-tiled object's tiling mode
against its associated fb modifier.

Cc: Tvrtko Ursulin 
Signed-off-by: Vivek Kasireddy 
---
tests/kms_addfb_basic.c | 9 +
1 file changed, 9 insertions(+)

diff --git a/tests/kms_addfb_basic.c b/tests/kms_addfb_basic.c
index d466e4d..7ca1add 100644
--- a/tests/kms_addfb_basic.c
+++ b/tests/kms_addfb_basic.c
@@ -373,6 +373,15 @@ static void addfb25_ytile(int fd, int gen)
f.handles[0] = gem_bo;
}

+igt_subtest("addfb25-Y-tiled-X-modifier-mismatch") {
+igt_require(gen >= 9);
+igt_require_fb_modifiers(fd);
+gem_set_tiling(fd, gem_bo, I915_TILING_Y, 1024*4);
+
+f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED;
+igt_assert(drmIoctl(fd,
LOCAL_DRM_IOCTL_MODE_ADDFB2, ) < 0 && errno == EINVAL);
+}
+
igt_subtest("addfb25-Y-tiled") {
igt_require_fb_modifiers(fd);



Wasn't the original WARN triggered by Y tiled object and Y fb
modifier?


Creating a new fb using a Y-tiled object and Y/X fb modifier will
not trigger the original WARN. It'll be triggered only if the fb
is going to be pinned -- and flipped. I am not sure how to get
that WARN to be triggered with the existing suite of igt tests.


Ah yes, you would need to attempt display it, not even necessarily
flip it. I am sure there are tests which do that. :) I know from
recent activity kms_rotation_crc for example creates a Y tiled FB
and displays it. So maybe borrow some code to start with from there.


Even better, kms_flip_tiling does the majority of what is needed here
already. Just drop in a subtest which will do set_tiling and that
should be good.


I have realized that I cannot get the the map_and_fenceable WARN to be
triggered with any of the IGT tests. This is because the WARN is
triggered only when pinning/fencing a Y-tiled fb that has a rotated
view (90/270 degree rotation) that none of the IGT tests can do. I
looked at and wrote a subtest in kms_rotation_crc but the WARN was not
triggered because IGT does not support atomic flip/commit yet.
Currently, since it does a SetPlane first, the object has a normal view
and its map_and-fenceable bit is set, however, when the rotation
property is applied, the object though has a rotated view, its
map_and-fenceable bit never gets updated and stays 1 and hence the
warning doesn't get triggered. I am copying the subtest code below for
reference.


It is possible and it looks like you have been on the right track. If 
you just avoided rendering to the fb (paint_squares) I think it might 
have worked.


Unfortunately before reading your e-mail thoroughly I rushed and 
developed my own POC so you will now see a patch series I copied you on.


That triggers the WARN nicely.

But your approach, localized changes to kms_rotation_crc, is probably 
better. So feel free to refine your test case and we can drop my patch 
series.


Regards,

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

Re: [Intel-gfx] [RFC 0/6] Non perf based Gen Graphics OA unit driver

[Robert Bragg]

On Fri, Oct 16, 2015 at 11:33 AM, Peter Zijlstra  wrote:
> On Fri, Oct 16, 2015 at 12:02:28PM +0200, Ingo Molnar wrote:
>>
>> * Peter Zijlstra  wrote:
>>
>> > > - We may be making some technical compromises a.t.m for the sake of
>> > >   using perf.
>> > >
>> > > perf_event_open() requires events to either relate to a pid or a
>> > > specific cpu core, while our device pmu relates to neither.  Events
>> > > opened with a pid will be automatically enabled/disabled according
>> > > to the scheduling of that process - so not appropriate for us.
>> >
>> > Right; the traditional cpu/pid mapping doesn't work well for devices;
>> > but maybe, with some work, we can create something like that
>> > global/local render context from it; although I've no clue what form
>> > that would need at this time.
>>
>> Could someone please help with some very basic questions, such as what the
>> hardware model of the 'OA' unit model is? How are OA registers set up, how 
>> are
>> their values made accessible to the host side, etc.
>
> Robert linked to:
>
>   
> https://01.org/sites/default/files/documentation/observability_performance_counters_haswell.pdf
>
> In a previous posting. It has some info, but full documentation, is as
> per the initial post, 'pending'.

There is now also some Broadwell documentation here:

https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-bdw-vol14-observability.pdf

Unfortunately though a mistake was made by the documentation team when
generating the PDF which unintentionally stripped out a lot of
information so it's not very helpful a.t.m. I've let them know about
some of the issues, but I'm not sure a.t.m when it may be updated.

I tried to fill in the gaps in some of our earlier conversations, so
maybe also go over those for more details too.

Otherwise the best reference is probably my code currently, either the
RFC patches I sent most recently which at least cover up to Haswell,
or the wip/rib/oa-next branch here: https://github.com/rib/linux. The
lastest perf based driver is currently in the archive/rib/oa-core-perf
branch for reference too.

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

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Only call commit_planes when there are things to commit.

[Ville Syrjälä]

On Wed, Sep 23, 2015 at 04:29:39PM +0200, Maarten Lankhorst wrote:
> The atomic helpers set planes_changed on a crtc_state if there is
> any plane_state bound to that crtc. If there's none and there is
> no pipe update required the crtc has nothing to update, so vblank
> evasion can be skipped.
> 
> Signed-off-by: Maarten Lankhorst 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index f64ea430b976..9754ee2bb37c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13146,7 +13146,8 @@ static int intel_atomic_commit(struct drm_device *dev,
>   if (!modeset)
>   intel_pre_plane_update(intel_crtc);
>  
> - if (crtc->state->active)
> + if (crtc->state->active &&
> + (crtc->state->planes_changed || update_pipe))
>   drm_atomic_helper_commit_planes_on_crtc(crtc_state);
>  
>   if (put_domains)

Series lgtm
Reviewed-by: Ville Syrjälä 

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

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

Re: [Intel-gfx] [PATCH i-g-t 1/3] lib/igt_fb: Allow specifiying object tiling when creating frame buffers

[Tvrtko Ursulin]

On 16/10/15 14:01, Ville Syrjälä wrote:

On Fri, Oct 16, 2015 at 01:54:51PM +0100, Tvrtko Ursulin wrote:


On 16/10/15 13:29, Ville Syrjälä wrote:

On Fri, Oct 16, 2015 at 01:19:35PM +0100, Tvrtko Ursulin wrote:


Hi,

On 16/10/15 13:03, Ville Syrjälä wrote:

On Fri, Oct 16, 2015 at 11:59:47AM +0100, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

Currently object tiling is inferred from the frame buffer modifier
and only for legacy X scanout.

It is useful to support overriding this selection for certain tests
so add the capability.


So you want to set up the object tiling differently from the fb
tiling? Why is that? And don't we reject it in the kernel? If we don't
need a fence for scanout (ie. FBC or gen2/3) we could allow it I
suppose, but not sure it it really helps with anything.


Hm, yes and no. Only different in a sense that currently igt_fb leaves
object tiling at linear regardless of the fb modifier tiling. (Apart for
the legacy X where it requires that they match.)

I needed a way of having Y tiled fb modifier and Y tiled object to hit a
warning in i915_gem_object_get_fence when only the rotated view exists.


Is there a problem of just doing what X tiled did also for Y tiled?


What do you mean? In the kernel or igt_fb? If latter then it needs to be
able to have object tiling as linear by default since that is how fb
modifiers were intended to be used - decoupled from obj tiling. Is that
what you meant?


Just setting obj to Y tiled when fb is Y tiled from igt_fb.


Right, so I explained that - we want to have Y tiled fb modifier + 
linear object in the majority of cases, _apart_ from this special test 
case which tries to hit a WARN_ON on !obj->map_and_fenceable. Which 
happens when obj tiling is Y, and fb tiling is Y, _and_ no normal VMA 
exists, just the rotate one.


Regards,

Tvrtko



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

Re: [Intel-gfx] [v2] drm/i915/skl: Init cdclk in the driver rather than relying on pre-os

[Kumar, Shobhit]

On 10/08/2015 05:54 PM, Ville Syrjälä wrote:

On Thu, Oct 08, 2015 at 05:43:41PM +0530, Kumar, Shobhit wrote:

On 10/08/2015 04:59 PM, Imre Deak wrote:

On to, 2015-10-08 at 09:58 +0530, Shobhit Kumar wrote:

Reuse what is programmed by pre-os, but in case there is no pre-os
initialization, init the cdclk with the max value by default untill
dynamic cdclk support comes.

v2: Check if BIOS programmed correctly rather than always calling init
  - Do validation of programmed cdctl and what it is expected
  - Only do slk_init_cdclk if validation failed else reuse BIOS
programmed value

Cc: Imre Deak 
Cc: Ville Syrjälä 
Signed-off-by: Shobhit Kumar 
---
   drivers/gpu/drm/i915/intel_ddi.c | 18 -
   drivers/gpu/drm/i915/intel_display.c | 39 
+++-
   2 files changed, 42 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 2d3cc82..3ec5618 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2946,11 +2946,19 @@ void intel_ddi_pll_init(struct drm_device *dev)
int cdclk_freq;

cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
-   dev_priv->skl_boot_cdclk = cdclk_freq;
-   if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
-   DRM_ERROR("LCPLL1 is disabled\n");
-   else
-   intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
+
+   /* Invalid CDCLK from BIOS ? */
+   if (cdclk_freq < 0) {
+   /* program to maximum cdclk till we have dynamic cdclk 
support */
+   dev_priv->skl_boot_cdclk = 675000;
+   skl_init_cdclk(dev_priv);


This would still try to reprogram CDCLK if BIOS enabled an output with
an incorrect CDCLK decimal part. Isn't this the exact scenario you're
seeing? As said before reprogramming CDCLK in that case would require
disabling the outputs first.


I went with the hypothesis that if VBIOS/GOP was loaded it would have to
correct the cdclock, with wrong decimal value display cannot be enabled.
For example AUX will fail on SKL. So for correct display output enabled
cdclock has to be correctly programmed. If it is wrong display was not
enabled at all.

The scenario which I am seeing is VBIOS/GOP is not loaded at all, and
the pre-os is not leaving cdclock in correct state.


But it still enabled DPLL0? Why does it do that if it doesn't set up
cdclk?



Okay digging more into FSP code, found a bug where even when display is 
not initialized in pre-os, still there was an initialization path for 
enabling display audio for HDMI which was enabling DPLL0. That also 
should have been not invoked when display itself is not being loaded. 
Not sure what going on in there !! Will work with FSP team and get this 
rectified. After that sanitize patch(following up this mail) can check 
the clock related programming and sanitize if needed as discussed with 
you Ville on IRC.


Regards
Shobhit





We would also need to call skl_init_cdclk if the BIOS hasn't enabled the
PLL for some reason. But I guess that could be a separate change.


+   } else {
+   dev_priv->skl_boot_cdclk = cdclk_freq;
+   if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
+   DRM_ERROR("LCPLL1 is disabled\n");
+   else
+   intel_display_power_get(dev_priv, 
POWER_DOMAIN_PLLS);
+   }
} else if (IS_BROXTON(dev)) {
broxton_init_cdclk(dev);
broxton_ddi_phy_init(dev);
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index bbeb6d3..f734410 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6634,12 +6634,15 @@ static int skylake_get_display_clock_speed(struct 
drm_device *dev)
uint32_t lcpll1 = I915_READ(LCPLL1_CTL);
uint32_t cdctl = I915_READ(CDCLK_CTL);
uint32_t linkrate;
+   int freq;

if (!(lcpll1 & LCPLL_PLL_ENABLE))
return 24000; /* 24MHz is the cd freq with NSSC ref */

-   if ((cdctl & CDCLK_FREQ_SEL_MASK) == CDCLK_FREQ_540)
-   return 54;
+   if ((cdctl & CDCLK_FREQ_SEL_MASK) == CDCLK_FREQ_540) {
+   freq = 54;
+   goto verify;
+   }

linkrate = (I915_READ(DPLL_CTRL1) &
DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0)) >> 1;
@@ -6649,30 +6652,46 @@ static int skylake_get_display_clock_speed(struct 
drm_device *dev)
/* vco 8640 */
switch (cdctl & CDCLK_FREQ_SEL_MASK) {
case CDCLK_FREQ_450_432:
-   return 432000;
+   freq = 432000;
+

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

[Chris Wilson]

On Fri, Oct 16, 2015 at 02:00:42PM +0100, Thomas Wood wrote:
> On 15 October 2015 at 09:05, Derek Morton  wrote:
> > This test is based on gem_exec_nop but submits nop batch buffers 
> > concurrently
> > from different threads to check for ring hangs and other issues during
> > concurrent submissions.
> 
> Is there any reason not to include this as extra subtests in
> gem_exec_nop so that related tests are grouped together?

It's a better fit for ringfill. It already does concurrent clients to
the same ring in order to catch exactly the same bugs as described here,
so extending it to fill all rings concurrently should be easy.
-Chris

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

Re: [Intel-gfx] [PATCH] drm/i915: Report context GTT size

[Chris Wilson]

On Wed, Oct 14, 2015 at 02:17:11PM +0100, Chris Wilson wrote:
> Since the beginning we have conflated the size of the global GTT with
> that of the per-process context sizes. In recent times (gen8+), those
> are no longer the same where the global GTT is limited to 2/4GiB but the
> per-process GTT may be anything up to 256TiB. Userspace knows nothing of
> this discrepancy and outside of one or two hacks, uses the getaperture
> ioctl to determine the maximum size it can use. Let's leave that as
> reporting the global GTT and use the context reporting method to
> describe the per-process value (which naturally fallsback to reporting
> the aliasing or global on older platforms, so userspace can always use
> this method where available).
> 
> Testcase: igt/gem_userptr_blits/minor-normal-sync
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90065
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 8 
>  include/uapi/drm/i915_drm.h | 5 +++--
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index 339a9d628f1e..cecb156c6f76 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -1087,6 +1087,14 @@ int i915_gem_context_getparam_ioctl(struct drm_device 
> *dev, void *data,
>   case I915_CONTEXT_PARAM_NO_ZEROMAP:
>   args->value = ctx->flags & CONTEXT_NO_ZEROMAP;
>   break;
> + case I915_CONTEXT_PARAM_GTT_SIZE:
> + if (ctx->ppgtt)
> + args->value = ctx->ppgtt->base.total;
> + else if (to_i915(dev)->mm.aliasing_ppgtt)
> + args->value = 
> to_i915(dev)->mm.aliasing_ppgtt->base.total;
> + else
> + args->value = to_i915(dev)->gtt.base.total;
> + break;
>   default:
>   ret = -EINVAL;
>   break;
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 83f60f01dca2..3f334967aa1b 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -1130,8 +1130,9 @@ struct drm_i915_gem_context_param {
>   __u32 ctx_id;
>   __u32 size;
>   __u64 param;
> -#define I915_CONTEXT_PARAM_BAN_PERIOD 0x1
> -#define I915_CONTEXT_PARAM_NO_ZEROMAP 0x2
> +#define I915_CONTEXT_PARAM_BAN_PERIOD0x1
> +#define I915_CONTEXT_PARAM_NO_ZEROMAP0x2
> +#define I915_CONTEXT_PARAM_GTT_SIZE  0x3
>   __u64 value;
>  };

Ping any takers? As ABI goes it is pretty simple...
-Chris

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

Re: [Intel-gfx] [PATCH v6] drm/i915: Add soft-pinning API for execbuffer

[Goel, Akash]

Discussed sometime back about this patch with Chris. He mainly has 2 
concerns with it.
1. The linear walk used by the patch to detect the overlapping objects 
would be expensive.
2. Restriction to disallow !RCS submissions for non-default contexts, 
which could lead to lot of conflicts for the placements, once multiple 
engines like media/blit/vebox are used


Both of them can be addressed by the subsequent patches.
Chris already has the patch ready to reduce the validation overhead with 
the use of rbtree and there should be no implications of allowing the 
non-default contexts for !RCS submissions in execbuffer.


In the standalone form, the patch looks good, so
Reviewed-by: "Akash Goel "

On 10/16/2015 4:29 PM, Thomas Daniel wrote:

From: Chris Wilson 

Userspace can pass in an offset that it presumes the object is located
at. The kernel will then do its utmost to fit the object into that
location. The assumption is that userspace is handling its own object
locations (for example along with full-ppgtt) and that the kernel will
rarely have to make space for the user's requests.

Signed-off-by: Chris Wilson 

v2: Fixed incorrect eviction found by Michal Winiarski - fix suggested by Chris
Wilson.  Fixed incorrect error paths causing crash found by Michal Winiarski.
(Not published externally)

v3: Rebased because of trivial conflict in object_bind_to_vm.  Fixed eviction
to allow eviction of soft-pinned objects when another soft-pinned object used
by a subsequent execbuffer overlaps reported by Michal Winiarski.
(Not published externally)

v4: Moved soft-pinned objects to the front of ordered_vmas so that they are
pinned first after an address conflict happens to avoid repeated conflicts in
rare cases (Suggested by Chris Wilson).  Expanded comment on
drm_i915_gem_exec_object2.offset to cover this new API.

v5: Added I915_PARAM_HAS_EXEC_SOFTPIN parameter for detecting this capability
(Kristian). Added check for multiple pinnings on eviction (Akash). Made sure
buffers are not considered misplaced without the user specifying
EXEC_OBJECT_SUPPORTS_48B_ADDRESS.  User must assume responsibility for any
addressing workarounds.  Updated object2.offset field comment again to clarify
NO_RELOC case (Chris).  checkpatch cleanup.

v6: Trivial rebase on latest drm-intel-nightly

Cc: Chris Wilson 
Cc: Akash Goel 
Cc: Vinay Belgaumkar 
Cc: Michal Winiarski 
Cc: Zou Nanhai 
Cc: Kristian Høgsberg 
Signed-off-by: Thomas Daniel 
---
  drivers/gpu/drm/i915/i915_dma.c|  3 ++
  drivers/gpu/drm/i915/i915_drv.h|  2 +
  drivers/gpu/drm/i915/i915_gem.c| 64 --
  drivers/gpu/drm/i915/i915_gem_evict.c  | 39 ++
  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 19 +++--
  include/uapi/drm/i915_drm.h| 12 --
  6 files changed, 113 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2336af9..824c6c3 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -170,6 +170,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
case I915_PARAM_HAS_RESOURCE_STREAMER:
value = HAS_RESOURCE_STREAMER(dev);
break;
+   case I915_PARAM_HAS_EXEC_SOFTPIN:
+   value = 1;
+   break;
default:
DRM_DEBUG("Unknown parameter %d\n", param->param);
return -EINVAL;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1396af9..73c3acf 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2816,6 +2816,7 @@ void i915_gem_vma_destroy(struct i915_vma *vma);
  #define PIN_UPDATE(1<<5)
  #define PIN_ZONE_4G   (1<<6)
  #define PIN_HIGH  (1<<7)
+#define PIN_OFFSET_FIXED   (1<<8)
  #define PIN_OFFSET_MASK (~4095)
  int __must_check
  i915_gem_object_pin(struct drm_i915_gem_object *obj,
@@ -3163,6 +3164,7 @@ int __must_check i915_gem_evict_something(struct 
drm_device *dev,
  unsigned long start,
  unsigned long end,
  unsigned flags);
+int __must_check i915_gem_evict_for_vma(struct i915_vma *target);
  int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle);

  /* belongs in i915_gem_gtt.h */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1e67484..c3453bd 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3450,30 +3450,50 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object 
*obj,
if (IS_ERR(vma))
goto err_unpin;

-

[Intel-gfx] [v3] drm/i915/skl: If needed sanitize bios programmed cdclk

[Shobhit Kumar]

Especially in cases where pre-os does not enable display, cdclk might
not be in sane state. During sanitization initialize cdclk with maximum
value till we get dynamic cdclk support.

v2: Check if BIOS programmed correctly rather than always calling init
- Do validation of programmed cdctl and what it is expected
- Only do slk_init_cdclk if validation failed else reuse BIOS
  programmed value

v3: Move the validation logic in a separate sanitize function (Ville)

Cc: Imre Deak 
Cc: Ville Syrjälä 
Signed-off-by: Shobhit Kumar 
---
 drivers/gpu/drm/i915/intel_ddi.c | 12 
 drivers/gpu/drm/i915/intel_display.c | 31 +++
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 3 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index b25e99a..86d43e6 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2949,10 +2949,14 @@ void intel_ddi_pll_init(struct drm_device *dev)
 
cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
dev_priv->skl_boot_cdclk = cdclk_freq;
-   if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
-   DRM_ERROR("LCPLL1 is disabled\n");
-   else
-   intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
+   if (skl_sanitize_cdclk(dev_priv))
+   DRM_DEBUG_KMS("Sanitized cdclk programmed by pre-os\n");
+   else {
+   if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
+   DRM_ERROR("LCPLL1 is disabled\n");
+   else
+   intel_display_power_get(dev_priv, 
POWER_DOMAIN_PLLS);
+   }
} else if (IS_BROXTON(dev)) {
broxton_init_cdclk(dev);
broxton_ddi_phy_init(dev);
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 5f37f84..98333d3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5784,6 +5784,37 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
DRM_ERROR("DBuf power enable timeout\n");
 }
 
+int skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
+{
+   uint32_t lcpll1 = I915_READ(LCPLL1_CTL);
+   uint32_t cdctl = I915_READ(CDCLK_CTL);
+   int freq = dev_priv->skl_boot_cdclk;
+
+   /* Is PLL enabled and locked ? */
+   if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK)))
+   goto sanitize;
+
+   /* DPLL okay; verify the cdclock
+*
+* Noticed in some instances that the freq selection is correct but
+* decimal part is programmed wrong from BIOS where pre-os does not
+* enable display. Verify the same as well.
+*/
+   if (cdctl == ((cdctl & CDCLK_FREQ_SEL_MASK) | skl_cdclk_decimal(freq)))
+   /* All well; nothing to sanitize */
+   return false;
+sanitize:
+   /*
+* As of now initialize with max cdclk till
+* we get dynamic cdclk support
+* */
+   dev_priv->skl_boot_cdclk = 675000;
+   skl_init_cdclk(dev_priv);
+
+   /* we did have to sanitize */
+   return true;
+}
+
 /* Adjust CDclk dividers to allow high res or save power if possible */
 static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0598932..ec10e6a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1152,6 +1152,7 @@ void broxton_ddi_phy_uninit(struct drm_device *dev);
 void bxt_enable_dc9(struct drm_i915_private *dev_priv);
 void bxt_disable_dc9(struct drm_i915_private *dev_priv);
 void skl_init_cdclk(struct drm_i915_private *dev_priv);
+int skl_sanitize_cdclk(struct drm_i915_private *dev_priv);
 void skl_uninit_cdclk(struct drm_i915_private *dev_priv);
 void intel_dp_get_m_n(struct intel_crtc *crtc,
  struct intel_crtc_state *pipe_config);
-- 
2.4.3

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

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Treat ringbuffer writes as write to normal memory

[Chris Wilson]

On Fri, Oct 16, 2015 at 06:06:09PM +0300, Ville Syrjälä wrote:
> On Thu, Oct 08, 2015 at 01:39:56PM +0100, Chris Wilson wrote:
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
> > b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 49aa52440db2..0eaaab92dea0 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -2180,13 +2180,8 @@ static int ring_wait_for_space(struct 
> > intel_engine_cs *ring, int n)
> >  
> >  static void __wrap_ring_buffer(struct intel_ringbuffer *ringbuf)
> >  {
> > -   uint32_t __iomem *virt;
> > int rem = ringbuf->size - ringbuf->tail;
> > -
> > -   virt = ringbuf->virtual_start + ringbuf->tail;
> > -   rem /= 4;
> > -   while (rem--)
> > -   iowrite32(MI_NOOP, virt++);
> > +   memset(ringbuf->virtual_start + ringbuf->tail, 0, rem);
> 
> Hmm. The lrc copy looks identical to this one. But looks like most of
> intel_lrc.c ring handling is copy pasted anyway, so there's even more
> that could be thrown out.

Yes, only this time I am trying to be sneaky and only delete small
chunks at a time. It may take a few years to do what could be done in a
morning, but whatever.
-Chris

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

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Treat ringbuffer writes as write to normal memory

[Ville Syrjälä]

On Thu, Oct 08, 2015 at 01:39:56PM +0100, Chris Wilson wrote:
> Ringbuffers are now being written to either through LLC or WC paths, so
> treating them as simply iomem is no longer adequate. However, for the
> older !llc hardware, the hardware is documentated as treating the TAIL
> register update as serialising, so we can relax the barriers when filling
> the rings (but even if it were not, it is still an uncached register write
> and so serialising anyway.).
> 
> For simplicity, let's ignore the iomem annotation.
> 
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/intel_lrc.c|  7 +--
>  drivers/gpu/drm/i915/intel_lrc.h|  6 +++---
>  drivers/gpu/drm/i915/intel_ringbuffer.c |  7 +--
>  drivers/gpu/drm/i915/intel_ringbuffer.h | 17 -
>  4 files changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> b/drivers/gpu/drm/i915/intel_lrc.c
> index d38746c5370d..10020505be75 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -713,13 +713,8 @@ intel_logical_ring_advance_and_submit(struct 
> drm_i915_gem_request *request)
>  
>  static void __wrap_ring_buffer(struct intel_ringbuffer *ringbuf)
>  {
> - uint32_t __iomem *virt;
>   int rem = ringbuf->size - ringbuf->tail;
> -
> - virt = ringbuf->virtual_start + ringbuf->tail;
> - rem /= 4;
> - while (rem--)
> - iowrite32(MI_NOOP, virt++);
> + memset(ringbuf->virtual_start + ringbuf->tail, 0, rem);
>  
>   ringbuf->tail = 0;
>   intel_ring_update_space(ringbuf);
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h 
> b/drivers/gpu/drm/i915/intel_lrc.h
> index 64f89f9982a2..8b56fb934763 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -53,8 +53,9 @@ int logical_ring_flush_all_caches(struct 
> drm_i915_gem_request *req);
>   */
>  static inline void intel_logical_ring_advance(struct intel_ringbuffer 
> *ringbuf)
>  {
> - ringbuf->tail &= ringbuf->size - 1;
> + intel_ringbuffer_advance(ringbuf);
>  }
> +
>  /**
>   * intel_logical_ring_emit() - write a DWORD to the ringbuffer.
>   * @ringbuf: Ringbuffer to write to.
> @@ -63,8 +64,7 @@ static inline void intel_logical_ring_advance(struct 
> intel_ringbuffer *ringbuf)
>  static inline void intel_logical_ring_emit(struct intel_ringbuffer *ringbuf,
>  u32 data)
>  {
> - iowrite32(data, ringbuf->virtual_start + ringbuf->tail);
> - ringbuf->tail += 4;
> + intel_ringbuffer_emit(ringbuf, data);
>  }
>  
>  /* Logical Ring Contexts */
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 49aa52440db2..0eaaab92dea0 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2180,13 +2180,8 @@ static int ring_wait_for_space(struct intel_engine_cs 
> *ring, int n)
>  
>  static void __wrap_ring_buffer(struct intel_ringbuffer *ringbuf)
>  {
> - uint32_t __iomem *virt;
>   int rem = ringbuf->size - ringbuf->tail;
> -
> - virt = ringbuf->virtual_start + ringbuf->tail;
> - rem /= 4;
> - while (rem--)
> - iowrite32(MI_NOOP, virt++);
> + memset(ringbuf->virtual_start + ringbuf->tail, 0, rem);

Hmm. The lrc copy looks identical to this one. But looks like most of
intel_lrc.c ring handling is copy pasted anyway, so there's even more
that could be thrown out.

This patch seems OK to me
Reviewed-by: Ville Syrjälä 

>  
>   ringbuf->tail = 0;
>   intel_ring_update_space(ringbuf);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
> b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 2e85fda94963..ff290765e6c9 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -427,17 +427,24 @@ int intel_ring_alloc_request_extras(struct 
> drm_i915_gem_request *request);
>  
>  int __must_check intel_ring_begin(struct drm_i915_gem_request *req, int n);
>  int __must_check intel_ring_cacheline_align(struct drm_i915_gem_request 
> *req);
> +static inline void intel_ringbuffer_emit(struct intel_ringbuffer *rb,
> +  u32 data)
> +{
> + *(uint32_t __force *)(rb->virtual_start + rb->tail) = data;
> + rb->tail += 4;
> +}
> +static inline void intel_ringbuffer_advance(struct intel_ringbuffer *rb)
> +{
> + rb->tail &= rb->size - 1;
> +}
>  static inline void intel_ring_emit(struct intel_engine_cs *ring,
>  u32 data)
>  {
> - struct intel_ringbuffer *ringbuf = ring->buffer;
> - iowrite32(data, ringbuf->virtual_start + ringbuf->tail);
> - ringbuf->tail += 4;
> + intel_ringbuffer_emit(ring->buffer, data);
>  }
>  static inline void intel_ring_advance(struct intel_engine_cs *ring)
>  {
> - struct intel_ringbuffer *ringbuf = ring->buffer;
> -   

[Intel-gfx] [PATCH v6 00/23] Color Management for DRM framework

[Shashank Sharma]

This patch set adds Color Manager implementation in DRM layer. Color Manager
is an extension in DRM framework to support color correction/enhancement.

Various Hardware platforms can support several color correction capabilities.
Color Manager provides abstraction of these capabilities and allows a user
space UI agent to correct/enhance the display using the DRM property interface.

How is this going to work?
==
1. This patch series adds a few new properties in DRM framework. These 
properties are:
a. color_capabilities property (type blob)
b. Color Transformation Matrix property for corrections like CSC 
(called CTM, type blob)
c. Palette correction properties for corrections like gamma fixup 
(called palette_correction, type blob)
2. Also, this patch series adds few structures to indicate specifications of a 
property like size, no_of_samples for correction etc.
3. These properties are present in mode_config.
4. When the platform's display driver loads, it fills up the values of 
color_capabilities property using the standard structures (added in step 2).
   For example, Intel's I915 driver adds following color correction 
capabilities:
a. gamma correction capability as palette correction property, with 257 
correction coefficients and a max/min value
b. csc correction capability as CTM correction property, with 3x3 
transformation matrix values and max/min values
5. Now when userspace comes up, it queries the platform's color capabilities by 
doing a get_property() on color_capabilities DRM property
6. Reading the blob, the userspace understands the color capabilities of the 
platform.
   For example, userspace will understand it can support:
a. palette_correction with 257 coefficients
b. CSC correction with 3x3 = 9 values
7. To set color correction values, userspace:
a. creates a blob using the create_blob_ioctl in standard 
palette_correction structure format, with the correction values
b. calls the set_property_ioctl with the blob_id as value for the 
property
8. Driver refers to the blob, gets the correction values and applies the 
correction in HW.
9. To get currently applied color correction values, userspace:
a. calls a get_property_ioctl on that color property
b. gets the blob_id for the currently applied correction from DRM 
infrastructure
c. gets the blob using get_blob_ioctl and hence the currently applied 
values

That's all! :)

About the patch series:
===
The patch series first adds the color management support in DRM layer.
Then it adds the color management framework in I915 layer.
After that, it implements platform specific core color correction functios.

Intel color manager registers color correction with DRM color manager in this 
way:
 - CSC transformation is registered as CTM DRM property
 - Gamma correction is registered as palette_after_ctm DRM property
 - Degamma correction is registered as palette_before_ctm DRM property

Our thanks to all the reviewers who have given valuable comments in terms of 
design
and implementation to our previous sets of patches. Special mention of thanks 
should
go to Matt Roper for all his inputs/suggestions in implementation of this 
module,
using DRM atomic CRTC commit path.

V2: Worked on review comments from Matt, Jim, Thierry, Rob.
V3: Worked on review comments from Matt, Jim, Rob:
 - Jim, Rob:
   
   Re-arranged the whole patch series in the sequence of features, currently:
   First 5 patches add color management support in DRM layer
   Next 7 patches add Intel color management framework in I915 driver
   Next 5 patches add color correction for CHV (gamma, degamma and CSC)
   Next 2 patches enable color management, by attaching the properties to 
CRTC(Matt)
   Next 4 patches add color correction for BDW (gamma, degamma)
 - Matt:
   =
   Patch 3: Added refernce/unreference for blob
   Patch 7: return -EINVAL and added debug message
   Patch 8: check for valid blob, from create blob
moved call to intel_crtc_attach_color_prop in the end of full 
implementation (CHV)
   Patch 9: DRM_ERROR->DRM_DEBUG for NULL blob case
   Patch 13: Added static for internal functions
   Patch 20-24: renamed gen9_* functions to bdw_*
   Added new variables in device_info structure num_samples_after_ctm and 
num_samples_before_ctm
   Added new function in patch 8 to load capabilities based on device_info 
across all platforms

V4: Worked on review comments from Daniel, Matt, Rob, Jim
 - Rob, Jim:
   =
   Patch 15, 22: Prepare CSC coeff properly(chv, bdw).
   Patch 13, 20: Gamma max should be (1<<24) -1(chv, bdw).
 - Daniel, Matt:
   =
   Patch 2: Create separate properties to query color capabilities, not a 
single blob.
   Patch 4, 5, 10: Add set/get property interface in DRM layer, not in I915 
layer.

V5: Addressed review comments from Emil, Rob.
 -Emil:
  ==
  Patch 3: Fixed 

[Intel-gfx] [PATCH v6 01/23] drm: Create Color Management DRM properties

[Shashank Sharma]

Color Management is an extension to DRM framework. It allows
abstraction of hardware color correction and enhancement capabilities
by virtue of DRM properties.

There are two major types of color correction supported by DRM
color manager:
- CTM: color transformation matrix, properties where a correction
   matrix is used for color correction.
- Palette correction: Where direct LUT values are sent to be applied
   on a color palette.

This patch initializes color management framework by:
1. Introducing new pointers in DRM mode_config structure to
   carry CTM and Palette color correction properties.
2. Creating these DRM properties in DRM standard properties creation
   sequence.

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal Malladi 
---
 drivers/gpu/drm/drm_crtc.c | 19 +++
 include/drm/drm_crtc.h |  5 +
 2 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 6058f4b..24094ee 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1471,6 +1471,25 @@ static int drm_mode_create_standard_properties(struct 
drm_device *dev)
return -ENOMEM;
dev->mode_config.prop_mode_id = prop;
 
+   /* Color Management properties */
+   prop = drm_property_create(dev,
+   DRM_MODE_PROP_BLOB, "PALETTE_AFTER_CTM", 0);
+   if (!prop)
+   return -ENOMEM;
+   dev->mode_config.cm_palette_after_ctm_property = prop;
+
+   prop = drm_property_create(dev,
+   DRM_MODE_PROP_BLOB, "PALETTE_BEFORE_CTM", 0);
+   if (!prop)
+   return -ENOMEM;
+   dev->mode_config.cm_palette_before_ctm_property = prop;
+
+   prop = drm_property_create(dev,
+   DRM_MODE_PROP_BLOB, "CTM", 0);
+   if (!prop)
+   return -ENOMEM;
+   dev->mode_config.cm_ctm_property = prop;
+
return 0;
 }
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 33ddedd..5ddc1a2 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1148,6 +1148,11 @@ struct drm_mode_config {
struct drm_property *suggested_x_property;
struct drm_property *suggested_y_property;
 
+   /* Color Management Properties */
+   struct drm_property *cm_palette_before_ctm_property;
+   struct drm_property *cm_palette_after_ctm_property;
+   struct drm_property *cm_ctm_property;
+
/* dumb ioctl parameters */
uint32_t preferred_depth, prefer_shadow;
 
-- 
1.9.1

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

[Intel-gfx] [PATCH v6 22/23] drm/i915: BDW: Pipe level CSC correction

[Shashank Sharma]

BDW/SKL/BXT support Color Space Conversion (CSC) using a 3x3 matrix
that needs to be programmed into respective CSC registers.

This patch does the following:
1. Adds the core function to program CSC correction values for
   BDW/SKL/BXT platform
2. Adds CSC correction macros/defines

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal Malladi 
Signed-off-by: Kumar, Kiran S 
---
 drivers/gpu/drm/i915/i915_reg.h|   7 ++
 drivers/gpu/drm/i915/intel_color_manager.c | 113 +
 drivers/gpu/drm/i915/intel_color_manager.h |   8 ++
 3 files changed, 128 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 39fbafc..9838afc 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8195,4 +8195,11 @@ enum skl_disp_power_wells {
(_PIPE3(pipe, PAL_PREC_GCMAX_A, PAL_PREC_GCMAX_B, PAL_PREC_GCMAX_C))
 
 
+/* BDW CSC correction */
+#define CSC_COEFF_A0x49010
+#define CSC_COEFF_B0x49110
+#define CSC_COEFF_C0x49210
+#define _PIPE_CSC_COEFF(pipe) \
+   (_PIPE3(pipe, CSC_COEFF_A, CSC_COEFF_B, CSC_COEFF_C))
+
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
b/drivers/gpu/drm/i915/intel_color_manager.c
index 3d792b2..08a2e4e 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -363,6 +363,117 @@ static int bdw_set_degamma(struct drm_device *dev,
return 0;
 }
 
+static uint32_t bdw_prepare_csc_coeff(int64_t coeff)
+{
+   uint32_t reg_val, ls_bit_pos, exponent_bits, sign_bit = 0;
+   int32_t mantissa;
+   uint64_t abs_coeff;
+
+   coeff = min_t(int64_t, coeff, BDW_CSC_COEFF_MAX_VAL);
+   coeff = max_t(int64_t, coeff, BDW_CSC_COEFF_MIN_VAL);
+
+   abs_coeff = abs(coeff);
+   if (abs_coeff < (BDW_CSC_COEFF_UNITY_VAL >> 3)) {
+   /* abs_coeff < 0.125 */
+   exponent_bits = 3;
+   ls_bit_pos = 19;
+   } else if (abs_coeff >= (BDW_CSC_COEFF_UNITY_VAL >> 3) &&
+   abs_coeff < (BDW_CSC_COEFF_UNITY_VAL >> 2)) {
+   /* abs_coeff >= 0.125 && val < 0.25 */
+   exponent_bits = 2;
+   ls_bit_pos = 20;
+   } else if (abs_coeff >= (BDW_CSC_COEFF_UNITY_VAL >> 2)
+   && abs_coeff < (BDW_CSC_COEFF_UNITY_VAL >> 1)) {
+   /* abs_coeff >= 0.25 && val < 0.5 */
+   exponent_bits = 1;
+   ls_bit_pos = 21;
+   } else if (abs_coeff >= (BDW_CSC_COEFF_UNITY_VAL >> 1)
+   && abs_coeff < BDW_CSC_COEFF_UNITY_VAL) {
+   /* abs_coeff >= 0.5 && val < 1.0 */
+   exponent_bits = 0;
+   ls_bit_pos = 22;
+   } else if (abs_coeff >= BDW_CSC_COEFF_UNITY_VAL &&
+   abs_coeff < (BDW_CSC_COEFF_UNITY_VAL << 1)) {
+   /* abs_coeff >= 1.0 && val < 2.0 */
+   exponent_bits = 7;
+   ls_bit_pos = 23;
+   } else {
+   /* abs_coeff >= 2.0 && val < 4.0 */
+   exponent_bits = 6;
+   ls_bit_pos = 24;
+   }
+
+   mantissa = GET_BITS_ROUNDOFF(abs_coeff, ls_bit_pos, CSC_MAX_VALS);
+   if (coeff < 0)
+   sign_bit = 1;
+
+   reg_val = 0;
+   SET_BITS(reg_val, exponent_bits, 12, 3);
+   SET_BITS(reg_val, mantissa, 3, 9);
+   SET_BITS(reg_val, sign_bit, 15, 1);
+   return reg_val;
+}
+
+static int bdw_set_csc(struct drm_device *dev, struct drm_property_blob *blob,
+   struct drm_crtc *crtc)
+{
+   enum pipe pipe;
+   enum plane plane;
+   int temp, word;
+   int count = 0;
+   u32 reg, plane_ctl, mode;
+   struct drm_ctm *csc_data;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+
+   if (WARN_ON(!blob))
+   return -EINVAL;
+
+   if (blob->length != sizeof(struct drm_ctm)) {
+   DRM_ERROR("Invalid length of data received\n");
+   return -EINVAL;
+   }
+
+   csc_data = (struct drm_ctm *)blob->data;
+   pipe = to_intel_crtc(crtc)->pipe;
+   plane = to_intel_crtc(crtc)->plane;
+
+   plane_ctl = I915_READ(PLANE_CTL(pipe, plane));
+   plane_ctl |= PLANE_CTL_PIPE_CSC_ENABLE;
+   I915_WRITE(PLANE_CTL(pipe, plane), plane_ctl);
+   reg = _PIPE_CSC_COEFF(pipe);
+
+   /*
+   * BDW CSC correction coefficients are written like this:
+   * first two values go in a pair, into first register(0:15 and 16:31)
+   * third one alone goes into second register (16:31). Same
+   * pattern repeats for 3 times = 3 * 3 = 9 values.
+   */
+   while (count < CSC_MAX_VALS) {
+   word = 0;
+   temp = bdw_prepare_csc_coeff(csc_data->ctm_coeff[count++]);
+   SET_BITS(word, temp, 16, 16);
+
+   

[Intel-gfx] [PATCH v6 18/23] drm/i915: BDW: Load gamma correction values

[Shashank Sharma]

I915 color manager registers pipe gamma correction as palette
correction after CTM property.

For BDW and higher platforms, split gamma correction is the best
gamma correction. This patch adds the no of coefficients(512) for
split gamma correction as "num_samples_after_ctm" parameter in device
info structures, for all of those.

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal Malladi 
---
 drivers/gpu/drm/i915/i915_drv.c| 7 +++
 drivers/gpu/drm/i915/intel_color_manager.h | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6adf002..8beac5c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -302,6 +302,7 @@ static const struct intel_device_info 
intel_broadwell_d_info = {
.gen = 8, .num_pipes = 3,
.need_gfx_hws = 1, .has_hotplug = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
+   .num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
.has_llc = 1,
.has_ddi = 1,
.has_fpga_dbg = 1,
@@ -314,6 +315,7 @@ static const struct intel_device_info 
intel_broadwell_m_info = {
.gen = 8, .is_mobile = 1, .num_pipes = 3,
.need_gfx_hws = 1, .has_hotplug = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
+   .num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
.has_llc = 1,
.has_ddi = 1,
.has_fpga_dbg = 1,
@@ -326,6 +328,7 @@ static const struct intel_device_info 
intel_broadwell_gt3d_info = {
.gen = 8, .num_pipes = 3,
.need_gfx_hws = 1, .has_hotplug = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
+   .num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
.has_llc = 1,
.has_ddi = 1,
.has_fpga_dbg = 1,
@@ -338,6 +341,7 @@ static const struct intel_device_info 
intel_broadwell_gt3m_info = {
.gen = 8, .is_mobile = 1, .num_pipes = 3,
.need_gfx_hws = 1, .has_hotplug = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
+   .num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
.has_llc = 1,
.has_ddi = 1,
.has_fpga_dbg = 1,
@@ -363,6 +367,7 @@ static const struct intel_device_info intel_skylake_info = {
.gen = 9, .num_pipes = 3,
.need_gfx_hws = 1, .has_hotplug = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
+   .num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
.has_llc = 1,
.has_ddi = 1,
.has_fpga_dbg = 1,
@@ -376,6 +381,7 @@ static const struct intel_device_info 
intel_skylake_gt3_info = {
.gen = 9, .num_pipes = 3,
.need_gfx_hws = 1, .has_hotplug = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
+   .num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
.has_llc = 1,
.has_ddi = 1,
.has_fpga_dbg = 1,
@@ -389,6 +395,7 @@ static const struct intel_device_info intel_broxton_info = {
.gen = 9,
.need_gfx_hws = 1, .has_hotplug = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
+   .num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
.num_pipes = 3,
.has_ddi = 1,
.has_fpga_dbg = 1,
diff --git a/drivers/gpu/drm/i915/intel_color_manager.h 
b/drivers/gpu/drm/i915/intel_color_manager.h
index 7b96512..271246a 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.h
+++ b/drivers/gpu/drm/i915/intel_color_manager.h
@@ -89,3 +89,6 @@
 #define CGM_GAMMA_EN   (1 << 2)
 #define CGM_CSC_EN (1 << 1)
 #define CGM_DEGAMMA_EN (1 << 0)
+
+/* Gamma on BDW */
+#define BDW_SPLITGAMMA_MAX_VALS512
-- 
1.9.1

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

[Intel-gfx] [PATCH v6 23/23] drm/i915: disable plane gamma

[Shashank Sharma]

In plane enabling sequence, plane gamma bit is by default enabled.
Plane gamma gets higher priority than pipe gamma, if both enabled.

This patch disables plane gamma from sequence. If required, plane
gamma can be enabled via the color manager drm interface.

signed-off-by: Kumar, Kiran S 
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 drivers/gpu/drm/i915/intel_sprite.c  | 7 ---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 61562a3..72b701a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2820,7 +2820,7 @@ static void ironlake_update_primary_plane(struct drm_crtc 
*crtc,
 
pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
 
-   dspcntr = DISPPLANE_GAMMA_ENABLE;
+   dspcntr = (DISPPLANE_GAMMA_ENABLE | PLANE_CTL_PLANE_GAMMA_DISABLE);
 
dspcntr |= DISPLAY_PLANE_ENABLE;
 
diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index 56dc132..6e2be1c 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -204,7 +204,8 @@ skl_update_plane(struct drm_plane *drm_plane, struct 
drm_crtc *crtc,
 
plane_ctl = PLANE_CTL_ENABLE |
PLANE_CTL_PIPE_GAMMA_ENABLE |
-   PLANE_CTL_PIPE_CSC_ENABLE;
+   PLANE_CTL_PIPE_CSC_ENABLE |
+   PLANE_CTL_PLANE_GAMMA_DISABLE;
 
plane_ctl |= skl_plane_ctl_format(fb->pixel_format);
plane_ctl |= skl_plane_ctl_tiling(fb->modifier[0]);
@@ -409,7 +410,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc 
*crtc,
 * Enable gamma to match primary/cursor plane behaviour.
 * FIXME should be user controllable via propertiesa.
 */
-   sprctl |= SP_GAMMA_ENABLE;
+   sprctl |= (SP_GAMMA_ENABLE | PLANE_CTL_PLANE_GAMMA_DISABLE);
 
if (obj->tiling_mode != I915_TILING_NONE)
sprctl |= SP_TILED;
@@ -528,7 +529,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc 
*crtc,
 * Enable gamma to match primary/cursor plane behaviour.
 * FIXME should be user controllable via propertiesa.
 */
-   sprctl |= SPRITE_GAMMA_ENABLE;
+   sprctl |= (SPRITE_GAMMA_ENABLE | PLANE_CTL_PLANE_GAMMA_DISABLE);
 
if (obj->tiling_mode != I915_TILING_NONE)
sprctl |= SPRITE_TILED;
-- 
1.9.1

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

[Intel-gfx] [PATCH v6 20/23] drm/i915: BDW: Load degamma correction values

[Shashank Sharma]

I915 color manager registers pipe degamma correction as palette
correction before CTM, DRM property.

This patch adds the no of coefficients(512) for degamma correction
as "num_samples_before_ctm" parameter in device info structures,
for BDW and higher platforms.

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal Malladi 
---
 drivers/gpu/drm/i915/i915_drv.c| 7 +++
 drivers/gpu/drm/i915/intel_color_manager.h | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8beac5c..1c68e91 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -303,6 +303,7 @@ static const struct intel_device_info 
intel_broadwell_d_info = {
.need_gfx_hws = 1, .has_hotplug = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
.num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
+   .num_samples_before_ctm = BDW_DEGAMMA_MAX_VALS,
.has_llc = 1,
.has_ddi = 1,
.has_fpga_dbg = 1,
@@ -316,6 +317,7 @@ static const struct intel_device_info 
intel_broadwell_m_info = {
.need_gfx_hws = 1, .has_hotplug = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
.num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
+   .num_samples_before_ctm = BDW_DEGAMMA_MAX_VALS,
.has_llc = 1,
.has_ddi = 1,
.has_fpga_dbg = 1,
@@ -329,6 +331,7 @@ static const struct intel_device_info 
intel_broadwell_gt3d_info = {
.need_gfx_hws = 1, .has_hotplug = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
.num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
+   .num_samples_before_ctm = BDW_DEGAMMA_MAX_VALS,
.has_llc = 1,
.has_ddi = 1,
.has_fpga_dbg = 1,
@@ -342,6 +345,7 @@ static const struct intel_device_info 
intel_broadwell_gt3m_info = {
.need_gfx_hws = 1, .has_hotplug = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
.num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
+   .num_samples_before_ctm = BDW_DEGAMMA_MAX_VALS,
.has_llc = 1,
.has_ddi = 1,
.has_fpga_dbg = 1,
@@ -368,6 +372,7 @@ static const struct intel_device_info intel_skylake_info = {
.need_gfx_hws = 1, .has_hotplug = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
.num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
+   .num_samples_before_ctm = BDW_DEGAMMA_MAX_VALS,
.has_llc = 1,
.has_ddi = 1,
.has_fpga_dbg = 1,
@@ -382,6 +387,7 @@ static const struct intel_device_info 
intel_skylake_gt3_info = {
.need_gfx_hws = 1, .has_hotplug = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
.num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
+   .num_samples_before_ctm = BDW_DEGAMMA_MAX_VALS,
.has_llc = 1,
.has_ddi = 1,
.has_fpga_dbg = 1,
@@ -396,6 +402,7 @@ static const struct intel_device_info intel_broxton_info = {
.need_gfx_hws = 1, .has_hotplug = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
.num_samples_after_ctm = BDW_SPLITGAMMA_MAX_VALS,
+   .num_samples_before_ctm = BDW_DEGAMMA_MAX_VALS,
.num_pipes = 3,
.has_ddi = 1,
.has_fpga_dbg = 1,
diff --git a/drivers/gpu/drm/i915/intel_color_manager.h 
b/drivers/gpu/drm/i915/intel_color_manager.h
index 6c7cb08..e0c486e 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.h
+++ b/drivers/gpu/drm/i915/intel_color_manager.h
@@ -98,3 +98,6 @@
 #define BDW_MAX_GAMMA ((1 << 24) - 1)
 #define BDW_INDEX_AUTO_INCREMENT   (1 << 15)
 #define BDW_INDEX_SPLIT_MODE   (1 << 31)
+
+/* Degamma on BDW */
+#define BDW_DEGAMMA_MAX_VALS   512
-- 
1.9.1

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

[Intel-gfx] [PATCH v6 13/23] drm/i915: CHV: Pipe level Gamma correction

[Shashank Sharma]

CHV/BSW platform supports two different pipe level gamma
correction modes, which are:
1. Legacy 8-bit mode
2. 10-bit CGM (Color Gamut Mapping) mode

This patch does the following:
1. Attaches Gamma property to CRTC
3. Adds the core Gamma correction function for CHV/BSW
4. Adds Gamma correction macros

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal Malladi 
---
 drivers/gpu/drm/i915/i915_reg.h| 12 
 drivers/gpu/drm/i915/intel_color_manager.c | 94 ++
 drivers/gpu/drm/i915/intel_color_manager.h | 13 +
 3 files changed, 119 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9ebf032..45ddd84 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8148,4 +8148,16 @@ enum skl_disp_power_wells {
 #define GEN9_VEBOX_MOCS_0  0xcb00  /* Video MOCS base register*/
 #define GEN9_BLT_MOCS_00xcc00  /* Blitter MOCS base register*/
 
+/* Color Management */
+#define PIPEA_CGM_CONTROL  (VLV_DISPLAY_BASE + 0x67A00)
+#define PIPEB_CGM_CONTROL  (VLV_DISPLAY_BASE + 0x69A00)
+#define PIPEC_CGM_CONTROL  (VLV_DISPLAY_BASE + 0x6BA00)
+#define PIPEA_CGM_GAMMA(VLV_DISPLAY_BASE + 0x67000)
+#define PIPEB_CGM_GAMMA(VLV_DISPLAY_BASE + 0x69000)
+#define PIPEC_CGM_GAMMA(VLV_DISPLAY_BASE + 0x6B000)
+#define _PIPE_CGM_CONTROL(pipe) \
+   (_PIPE3(pipe, PIPEA_CGM_CONTROL, PIPEB_CGM_CONTROL, PIPEC_CGM_CONTROL))
+#define _PIPE_GAMMA_BASE(pipe) \
+   (_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA))
+
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
b/drivers/gpu/drm/i915/intel_color_manager.c
index 334bfff..acb9647 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -27,6 +27,92 @@
 
 #include "intel_color_manager.h"
 
+static int chv_set_gamma(struct drm_device *dev, struct drm_property_blob 
*blob,
+   struct drm_crtc *crtc)
+{
+   enum pipe pipe;
+   u16 red_fract, green_fract, blue_fract;
+   u32 red, green, blue, num_samples;
+   u32 word = 0;
+   u32 count, cgm_gamma_reg, cgm_control_reg;
+   struct drm_r32g32b32 *correction_values;
+   struct drm_palette *gamma_data;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct drm_crtc_state *state = crtc->state;
+
+   if (WARN_ON(!blob))
+   return -EINVAL;
+
+   gamma_data = (struct drm_palette *)blob->data;
+   pipe = to_intel_crtc(crtc)->pipe;
+   num_samples = blob->length / sizeof(struct drm_r32g32b32);
+
+   switch (num_samples) {
+   case GAMMA_DISABLE_VALS:
+
+   /* Disable Gamma functionality on Pipe - CGM Block */
+   cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
+   cgm_control_reg &= ~CGM_GAMMA_EN;
+   I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
+   state->palette_after_ctm_blob = NULL;
+   DRM_DEBUG_DRIVER("Gamma disabled on Pipe %c\n",
+   pipe_name(pipe));
+   return 0;
+
+   case CHV_8BIT_GAMMA_MAX_VALS:
+   case CHV_10BIT_GAMMA_MAX_VALS:
+
+   count = 0;
+   cgm_gamma_reg = _PIPE_GAMMA_BASE(pipe);
+   correction_values = gamma_data->lut;
+
+   while (count < num_samples) {
+   blue = correction_values[count].b32;
+   green = correction_values[count].g32;
+   red = correction_values[count].r32;
+
+   if (blue > CHV_MAX_GAMMA)
+   blue = CHV_MAX_GAMMA;
+
+   if (green > CHV_MAX_GAMMA)
+   green = CHV_MAX_GAMMA;
+
+   if (red > CHV_MAX_GAMMA)
+   red = CHV_MAX_GAMMA;
+
+   /* get MSB 10 bits from fraction part (14:23) */
+   blue_fract = GET_BITS(blue, 14, 10);
+   green_fract = GET_BITS(green, 14, 10);
+   red_fract = GET_BITS(red, 14, 10);
+
+   /* Green (25:16) and Blue (9:0) to be written */
+   SET_BITS(word, green_fract, 16, 10);
+   SET_BITS(word, blue_fract, 0, 10);
+   I915_WRITE(cgm_gamma_reg, word);
+   cgm_gamma_reg += 4;
+
+   /* Red (9:0) to be written */
+   word = red_fract;
+   I915_WRITE(cgm_gamma_reg, word);
+
+   cgm_gamma_reg += 4;
+   count++;
+   }
+
+   /* Enable (CGM) Gamma on Pipe */
+   I915_WRITE(_PIPE_CGM_CONTROL(pipe),
+

[Intel-gfx] [PATCH v6 21/23] drm/i915: BDW: Pipe level degamma correction

[Shashank Sharma]

BDW/SKL/BXT supports Degamma color correction feature, which
linearizes the non-linearity due to gamma encoded color values.
This will be applied before Color Transformation.

This patch does the following:
1. Adds the core function to program DeGamma correction values for
   BDW/SKL/BXT platform
2. Adds DeGamma correction macros/defines

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal Malladi 
---
 drivers/gpu/drm/i915/intel_color_manager.c | 59 ++
 1 file changed, 59 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
b/drivers/gpu/drm/i915/intel_color_manager.c
index 44f99be..3d792b2 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -306,6 +306,63 @@ static int bdw_set_gamma(struct drm_device *dev, struct 
drm_property_blob *blob,
return 0;
 }
 
+static int bdw_set_degamma(struct drm_device *dev,
+   struct drm_property_blob *blob, struct drm_crtc *crtc)
+{
+   enum pipe pipe;
+   int num_samples;
+   u32 index, mode;
+   u32 pal_prec_index, pal_prec_data;
+   struct drm_palette *degamma_data;
+   struct drm_crtc_state *state = crtc->state;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct drm_r32g32b32 *correction_values = NULL;
+
+   if (WARN_ON(!blob))
+   return -EINVAL;
+
+   degamma_data = (struct drm_palette *)blob->data;
+   pipe = to_intel_crtc(crtc)->pipe;
+   num_samples = blob->length / sizeof(struct drm_r32g32b32);
+
+   switch (num_samples) {
+   case GAMMA_DISABLE_VALS:
+   /* Disable degamma on Pipe */
+   mode = I915_READ(GAMMA_MODE(pipe)) & ~GAMMA_MODE_MODE_MASK;
+   I915_WRITE(GAMMA_MODE(pipe), mode | GAMMA_MODE_MODE_8BIT);
+
+   state->palette_before_ctm_blob = NULL;
+   DRM_DEBUG_DRIVER("Disabling degamma on Pipe %c\n",
+   pipe_name(pipe));
+   break;
+
+   case BDW_SPLITGAMMA_MAX_VALS:
+   pal_prec_index = _PREC_PAL_INDEX(pipe);
+   pal_prec_data = _PREC_PAL_DATA(pipe);
+   correction_values = degamma_data->lut;
+
+   index = I915_READ(pal_prec_index);
+   index |= BDW_INDEX_AUTO_INCREMENT | BDW_INDEX_SPLIT_MODE;
+   I915_WRITE(pal_prec_index, index);
+
+   bdw_write_10bit_gamma_precision(dev, correction_values,
+   pal_prec_data, BDW_SPLITGAMMA_MAX_VALS);
+
+   /* Enable degamma on Pipe */
+   mode = I915_READ(GAMMA_MODE(pipe));
+   mode &= ~GAMMA_MODE_MODE_MASK;
+   I915_WRITE(GAMMA_MODE(pipe), mode | GAMMA_MODE_MODE_SPLIT);
+   DRM_DEBUG_DRIVER("degamma correction enabled on Pipe %c\n",
+   pipe_name(pipe));
+   break;
+
+   default:
+   DRM_ERROR("Invalid number of samples\n");
+   return -EINVAL;
+   }
+   return 0;
+}
+
 static s32 chv_prepare_csc_coeff(s64 csc_value)
 {
s32 csc_int_value;
@@ -596,6 +653,8 @@ void intel_color_manager_crtc_commit(struct drm_device *dev,
/* Degamma correction */
if (IS_CHERRYVIEW(dev))
ret = chv_set_degamma(dev, blob, crtc);
+   else if (IS_BROADWELL(dev) || IS_GEN9(dev))
+   ret = bdw_set_degamma(dev, blob, crtc);
 
if (ret)
DRM_ERROR("set degamma correction failed\n");
-- 
1.9.1

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

[Intel-gfx] [PATCH v6 11/23] drm/i915: CHV: Load gamma color correction values

[Shashank Sharma]

DRM color manager allows the driver to showcase its best color
correction capabilities using the specific query property
cm_coeff_after_ctm_property. The driver must loads the no. of
coefficients for color correction as per the platform capability
during the init time.

This patch adds no of coefficitents for best gamma color correction
modes possible in CHV, in device info structure, which is:
Gamma(10 bit, CGM HW unit): 257 coeff

These values will be loaded in cm_crtc_palette_capabilities_property
during the CRTC init section, by color manager's attach function.

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal Malladi 
---
 drivers/gpu/drm/i915/i915_drv.c| 2 ++
 drivers/gpu/drm/i915/intel_color_manager.h | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 760e0ce..7780de4 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -34,6 +34,7 @@
 #include "i915_drv.h"
 #include "i915_trace.h"
 #include "intel_drv.h"
+#include "intel_color_manager.h"
 
 #include 
 #include 
@@ -349,6 +350,7 @@ static const struct intel_device_info intel_cherryview_info 
= {
.gen = 8, .num_pipes = 3,
.need_gfx_hws = 1, .has_hotplug = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
+   .num_samples_after_ctm = CHV_10BIT_GAMMA_MAX_VALS,
.is_valleyview = 1,
.display_mmio_offset = VLV_DISPLAY_BASE,
GEN_CHV_PIPEOFFSETS,
diff --git a/drivers/gpu/drm/i915/intel_color_manager.h 
b/drivers/gpu/drm/i915/intel_color_manager.h
index eec52a7..a378fe1 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.h
+++ b/drivers/gpu/drm/i915/intel_color_manager.h
@@ -48,3 +48,6 @@
CLEAR_BITS(target, start_bit, no_bits); \
target |= (bit_pattern << start_bit);  \
} while (0)
+
+/* CHV */
+#define CHV_10BIT_GAMMA_MAX_VALS   257
-- 
1.9.1

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

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

[Morton, Derek J]

>
>On Fri, Oct 16, 2015 at 02:00:42PM +0100, Thomas Wood wrote:
>> On 15 October 2015 at 09:05, Derek Morton  wrote:
>> > This test is based on gem_exec_nop but submits nop batch buffers
>> > concurrently from different threads to check for ring hangs and
>> > other issues during concurrent submissions.
>>
>> Is there any reason not to include this as extra subtests in
>> gem_exec_nop so that related tests are grouped together?
>

gem_exec_nop takes 25 minutes to run on android (with the speed improvement 
patch I submitted yesterday) gem_exec_nop_concurrent takes a similar time. I 
would strongly resist combining the two into a test that takes nearly an hour 
for us to run.

>It's a better fit for ringfill. It already does concurrent clients to the same 
>ring in order to catch exactly the same bugs as described here, so extending 
>it to fill all rings concurrently should be easy.

I will take a look at gem_ringfill

//Derek

>-Chris
>
>--
>Chris Wilson, Intel Open Source Technology Centre
>

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

[Intel-gfx] [PATCH v6 02/23] drm: Create Color Management query properties

[Shashank Sharma]

DRM color management is written to extract the color correction
capabilities of various platforms, and every platform can showcase
its capabilities using the query properties.

Different hardwares can have different no of coefficients for palette
correction. Also the correction can be applied after/before color
transformation (CTM) unit in the display pipeline.

This patch adds two new read-only properties,
  - cm_coeff_before_ctm_property: A platform driver should use this
property to show supported no_of_coefficients for palette correction,
which gets applied before ctm correction.
  - cm_coeff_after_ctm_property: A platform driver should use this property
to show supported no_of_coefficients for palette correction, which gets
applied after ctm correction.

Signed-off-by: Shashank Sharma 
---
 drivers/gpu/drm/drm_crtc.c | 13 +
 include/drm/drm_crtc.h |  4 
 2 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 24094ee..2ee6ec3 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1490,6 +1490,19 @@ static int drm_mode_create_standard_properties(struct 
drm_device *dev)
return -ENOMEM;
dev->mode_config.cm_ctm_property = prop;
 
+   /* DRM properties to query color capabilities */
+   prop = drm_property_create(dev, DRM_MODE_PROP_IMMUTABLE,
+   "COEFFICIENTS_BEFORE_CTM", 0);
+   if (!prop)
+   return -ENOMEM;
+   dev->mode_config.cm_coeff_before_ctm_property = prop;
+
+   prop = drm_property_create(dev, DRM_MODE_PROP_IMMUTABLE,
+   "COEFFICIENTS_AFTER_CTM", 0);
+   if (!prop)
+   return -ENOMEM;
+   dev->mode_config.cm_coeff_after_ctm_property = prop;
+
return 0;
 }
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 5ddc1a2..1a56596 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1153,6 +1153,10 @@ struct drm_mode_config {
struct drm_property *cm_palette_after_ctm_property;
struct drm_property *cm_ctm_property;
 
+   /* Color management capabilities query */
+   struct drm_property *cm_coeff_before_ctm_property;
+   struct drm_property *cm_coeff_after_ctm_property;
+
/* dumb ioctl parameters */
uint32_t preferred_depth, prefer_shadow;
 
-- 
1.9.1

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

[Intel-gfx] [PATCH v6 03/23] drm: Add color correction blobs in CRTC state

[Shashank Sharma]

This patch adds new variables in CRTC state, to hold respective color
correction blobs. These blobs will be required during the atomic commit
for writing the color correction values in correction registers.

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal Malladi 
---
 drivers/gpu/drm/drm_atomic_helper.c | 12 
 include/drm/drm_crtc.h  |  5 +
 2 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 87a2a44..d73ca9b9 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2193,6 +2193,12 @@ void __drm_atomic_helper_crtc_duplicate_state(struct 
drm_crtc *crtc,
 
if (state->mode_blob)
drm_property_reference_blob(state->mode_blob);
+   if (state->ctm_blob)
+   drm_property_reference_blob(state->ctm_blob);
+   if (state->palette_after_ctm_blob)
+   drm_property_reference_blob(state->palette_after_ctm_blob);
+   if (state->palette_before_ctm_blob)
+   drm_property_reference_blob(state->palette_before_ctm_blob);
state->mode_changed = false;
state->active_changed = false;
state->planes_changed = false;
@@ -2238,6 +2244,12 @@ void __drm_atomic_helper_crtc_destroy_state(struct 
drm_crtc *crtc,
 {
if (state->mode_blob)
drm_property_unreference_blob(state->mode_blob);
+   if (state->ctm_blob)
+   drm_property_unreference_blob(state->ctm_blob);
+   if (state->palette_after_ctm_blob)
+   drm_property_unreference_blob(state->palette_after_ctm_blob);
+   if (state->palette_before_ctm_blob)
+   drm_property_unreference_blob(state->palette_before_ctm_blob);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 1a56596..d416e20 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -304,6 +304,11 @@ struct drm_crtc_state {
/* blob property to expose current mode to atomic userspace */
struct drm_property_blob *mode_blob;
 
+   /* blob properties to hold the color properties' blobs */
+   struct drm_property_blob *palette_before_ctm_blob;
+   struct drm_property_blob *palette_after_ctm_blob;
+   struct drm_property_blob *ctm_blob;
+
struct drm_pending_vblank_event *event;
 
struct drm_atomic_state *state;
-- 
1.9.1

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

[Intel-gfx] [PATCH v6 12/23] drm/i915: CHV: Load degamma color correction values

[Shashank Sharma]

DRM color manager allows the driver to showcase its best color
correction capabilities using the specific query property
cm_coeff_before_ctm_property. The driver must loads the no. of
coefficients for color correction as per the platform capability
during the init time.

This patch adds no of coefficitents for degamma color correction
modes possible in CHV, in device info structure, which is:
CGM Degamma(10 bit, CGM HW unit): 65 coeff

These values will be loaded in cm_crtc_palette_capabilities_property
during the CRTC init section, by color manager's attach function.

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal Malladi 
---
 drivers/gpu/drm/i915/i915_drv.c| 1 +
 drivers/gpu/drm/i915/intel_color_manager.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 7780de4..6adf002 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -351,6 +351,7 @@ static const struct intel_device_info intel_cherryview_info 
= {
.need_gfx_hws = 1, .has_hotplug = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
.num_samples_after_ctm = CHV_10BIT_GAMMA_MAX_VALS,
+   .num_samples_before_ctm = CHV_DEGAMMA_MAX_VALS,
.is_valleyview = 1,
.display_mmio_offset = VLV_DISPLAY_BASE,
GEN_CHV_PIPEOFFSETS,
diff --git a/drivers/gpu/drm/i915/intel_color_manager.h 
b/drivers/gpu/drm/i915/intel_color_manager.h
index a378fe1..14a1309 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.h
+++ b/drivers/gpu/drm/i915/intel_color_manager.h
@@ -51,3 +51,4 @@
 
 /* CHV */
 #define CHV_10BIT_GAMMA_MAX_VALS   257
+#define CHV_DEGAMMA_MAX_VALS   65
-- 
1.9.1

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

[Intel-gfx] [PATCH v6 06/23] drm: Add drm structures for palette color property

[Shashank Sharma]

This patch adds new structures in DRM layer for Palette color
correction.These structures will be used by user space agents
to configure appropriate number of samples and Palette LUT for
a platform.

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal Malladi 
---
 include/uapi/drm/drm.h | 20 
 1 file changed, 20 insertions(+)

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 3801584..3dce251 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -829,6 +829,26 @@ struct drm_event_vblank {
__u32 reserved;
 };
 
+struct drm_r32g32b32 {
+   /*
+* Data is in U8.24 fixed point format.
+* All platforms support values within [0, 1.0] range,
+* for Red, Green and Blue colors.
+*/
+   __u32 r32;
+   __u32 g32;
+   __u32 b32;
+   __u32 reserved;
+};
+
+struct drm_palette {
+   /*
+* Starting of palette LUT in R32G32B32 format.
+* Each of RGB value is in U8.24 fixed point format.
+*/
+   struct drm_r32g32b32 lut[0];
+};
+
 /* typedef area */
 #ifndef __KERNEL__
 typedef struct drm_clip_rect drm_clip_rect_t;
-- 
1.9.1

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

[Intel-gfx] [PATCH v6 08/23] drm/i915: Add set property interface for CRTC

[Shashank Sharma]

This patch adds set property interface for intel CRTC. This
interface will be used for set operation on any DRM properties.

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal Malladi 
---
 drivers/gpu/drm/i915/intel_display.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 5f37f84..7cad341 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13241,6 +13241,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
.page_flip = intel_crtc_page_flip,
.atomic_duplicate_state = intel_crtc_duplicate_state,
.atomic_destroy_state = intel_crtc_destroy_state,
+   .set_property = drm_atomic_helper_crtc_set_property,
 };
 
 static bool ibx_pch_dpll_get_hw_state(struct drm_i915_private *dev_priv,
-- 
1.9.1

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

[Intel-gfx] [PATCH v6 04/23] drm: Add set property support for color manager

[Shashank Sharma]

As per DRM color manager design, if a userspace wants to set a correction
blob, it prepares it and sends the blob_id to kernel via set_property
call. DRM framework takes this blob_id, gets the blob, and saves it
in the CRTC state, so that, during the atomic_commit, the color correction
values from the blob can referred and applied on display controller
registers.

This patch adds this set_property support for color correction blobs
in drm framework.

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal malladi 
---
 drivers/gpu/drm/drm_atomic.c | 53 ++--
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 7bb3845..12a34e9 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -390,6 +390,38 @@ int drm_atomic_set_mode_prop_for_crtc(struct 
drm_crtc_state *state,
 EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc);
 
 /**
+ * drm_atomic_crtc_set_blob - find and set a blob
+ * @state_blob: reference pointer to the color blob in the crtc_state
+ * @blob_id: blob_id coming from set_property() call
+ *
+ * Set a color correction blob (originating from a set blob property) on the
+ * desired CRTC state. This function will take reference of the blob property
+ * in the CRTC state, finds the blob based on blob_id (which comes from
+ * set_property call) and set the blob at the proper place.
+ *
+ * RETURNS:
+ * Zero on success, error code on failure.
+ */
+static int drm_atomic_crtc_set_blob(struct drm_device *dev,
+   struct drm_property_blob **state_blob, uint32_t blob_id)
+{
+   struct drm_property_blob *blob;
+
+   blob = drm_property_lookup_blob(dev, blob_id);
+   if (!blob) {
+   DRM_DEBUG_KMS("Invalid Blob ID\n");
+   return -EINVAL;
+   }
+
+   if (*state_blob)
+   drm_property_unreference_blob(*state_blob);
+
+   /* Attach the blob to be committed in state */
+   *state_blob = blob;
+   return 0;
+}
+
+/**
  * drm_atomic_crtc_set_property - set property on CRTC
  * @crtc: the drm CRTC to set a property on
  * @state: the state object to update with the new property value
@@ -422,8 +454,25 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
if (mode)
drm_property_unreference_blob(mode);
return ret;
-   }
-   else if (crtc->funcs->atomic_set_property)
+   } else if (property == config->cm_palette_after_ctm_property) {
+   ret = drm_atomic_crtc_set_blob(dev,
+   >palette_after_ctm_blob, val);
+   if (ret)
+   DRM_ERROR("Failed to load blob palette_after_ctm\n");
+   return ret;
+   } else if (property == config->cm_palette_before_ctm_property) {
+   ret = drm_atomic_crtc_set_blob(dev,
+   >palette_before_ctm_blob, val);
+   if (ret)
+   DRM_ERROR("Failed to load blob palette_before_ctm\n");
+   return ret;
+   } else if (property == config->cm_ctm_property) {
+   ret = drm_atomic_crtc_set_blob(dev,
+   >ctm_blob, val);
+   if (ret)
+   DRM_ERROR("Failed to load blob ctm\n");
+   return ret;
+   } else if (crtc->funcs->atomic_set_property)
return crtc->funcs->atomic_set_property(crtc, state, property, 
val);
else
return -EINVAL;
-- 
1.9.1

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

[Intel-gfx] [PATCH v6 07/23] drm: Add structure for CTM color property

[Shashank Sharma]

Color Manager framework defines a DRM property for color
space transformation and Gamut mapping. This property is called
CTM (Color Transformation Matrix).

This patch adds a new structure in DRM layer for CTM.
This structure can be used by all user space agents to
configure CTM coefficients for color correction.

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal Malladi 
---
 include/uapi/drm/drm.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 3dce251..d4de772 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -849,6 +849,16 @@ struct drm_palette {
struct drm_r32g32b32 lut[0];
 };
 
+struct drm_ctm {
+   /*
+* Each value is in S31.32 format.
+* This is 3x3 matrix in row major format.
+* Integer part will be clipped to nearest
+* max/min boundary as supported by the HW platform.
+*/
+   __s64 ctm_coeff[9];
+};
+
 /* typedef area */
 #ifndef __KERNEL__
 typedef struct drm_clip_rect drm_clip_rect_t;
-- 
1.9.1

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

[Intel-gfx] [PATCH v6 09/23] drm/i915: Create color management files

[Shashank Sharma]

This patch create new files intel_color_manager.c which
will contain the core color correction code for I915 driver
and its header intel_color_manager.h

The per color property patches coming up in this patch series
will fill the appropriate functions in this file.

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal Malladi 
---
 drivers/gpu/drm/i915/Makefile  |  3 +-
 drivers/gpu/drm/i915/intel_color_manager.c | 33 
 drivers/gpu/drm/i915/intel_color_manager.h | 50 ++
 drivers/gpu/drm/i915/intel_drv.h   |  3 ++
 4 files changed, 88 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/i915/intel_color_manager.c
 create mode 100644 drivers/gpu/drm/i915/intel_color_manager.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 44d290a..56caf9e 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -64,7 +64,8 @@ i915-y += intel_audio.o \
  intel_overlay.o \
  intel_psr.o \
  intel_sideband.o \
- intel_sprite.o
+ intel_sprite.o \
+ intel_color_manager.o
 i915-$(CONFIG_ACPI)+= intel_acpi.o intel_opregion.o
 i915-$(CONFIG_DRM_FBDEV_EMULATION) += intel_fbdev.o
 
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
b/drivers/gpu/drm/i915/intel_color_manager.c
new file mode 100644
index 000..b03ee94
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -0,0 +1,33 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ * Shashank Sharma 
+ * Kausal Malladi 
+ */
+
+#include "intel_color_manager.h"
+
+void intel_attach_color_properties_to_crtc(struct drm_device *dev,
+   struct drm_crtc *crtc)
+{
+}
diff --git a/drivers/gpu/drm/i915/intel_color_manager.h 
b/drivers/gpu/drm/i915/intel_color_manager.h
new file mode 100644
index 000..eec52a7
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_color_manager.h
@@ -0,0 +1,50 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ * Shashank Sharma 
+ * Kausal Malladi 
+ */
+#include 
+#include 
+#include "i915_drv.h"
+
+/* Color management bit utilities */
+#define GET_BIT_MASK(n) ((1 << n) - 1)
+
+/* Read bits of a word from bit no. 'start'(lsb) till 'n' bits */
+#define GET_BITS(x, start, nbits) ((x >> start) & GET_BIT_MASK(nbits))
+
+/* Round off by adding 1 to the immediate lower bit */
+#define GET_BITS_ROUNDOFF(x, start, nbits) \
+   ((GET_BITS(x, start, (nbits + 1)) + 1) >> 1)
+
+/* Clear bits of a word from bit no. 'start' till nbits */

Re: [Intel-gfx] [PATCH 2/4] drm/i915: Improve dynamic management/eviction of lrc backing objects

[Nick Hoath]

On 08/10/2015 14:35, Chris Wilson wrote:

On Wed, Oct 07, 2015 at 06:05:46PM +0200, Daniel Vetter wrote:

On Tue, Oct 06, 2015 at 03:52:02PM +0100, Nick Hoath wrote:

Shovel all context related objects through the active queue and obj
management.

- Added callback in vma_(un)bind to add CPU (un)mapping at same time
   if desired
- Inserted LRC hw context & ringbuf to vma active list

Issue: VIZ-4277
Signed-off-by: Nick Hoath 
---
  drivers/gpu/drm/i915/i915_drv.h |  4 ++
  drivers/gpu/drm/i915/i915_gem.c |  3 ++
  drivers/gpu/drm/i915/i915_gem_gtt.c |  8 
  drivers/gpu/drm/i915/intel_lrc.c| 28 +++--
  drivers/gpu/drm/i915/intel_ringbuffer.c | 71 ++---
  drivers/gpu/drm/i915/intel_ringbuffer.h |  3 --
  6 files changed, 79 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3d217f9..d660ee3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2169,6 +2169,10 @@ struct drm_i915_gem_object {
struct work_struct *work;
} userptr;
};
+
+   /** Support for automatic CPU side mapping of object */
+   int (*mmap)(struct drm_i915_gem_object *obj, bool unmap);


I don't think we need a map hook, that can still be done (if not done so
I disagree - this keeps the interface symmetrical. Searching for the 
do/undo code paths and finding they are in difference places, called via 
different routes makes code harder to follow.

already) by the callers. Also it's better to rename this to vma_unbind
(and it should be at the vma level I think) since there's other potential
Nope - the obj is created first, at a point where the map/unamp function 
can be known. Moving the map/unmap to the vma would mean having a 
callback path to the object just to set up the callback path when the 
vma is created anonymously at some later point.

users. So explicit maping, lazy unmapping for the kmaps we need. That's
the same design we're using for binding objects into gpu address spaces.

Also Chris Wilson has something similar, please align with him on the
precise design of this callback.


We need the unbind hook because of the movement in the first patch (it
is a separate issue, the code should work without it albeit having to
remap the ring/context state more often). The changelog in this patch
simply explains the i915_vma_move_to_active() additions. But to get the
shrink accurate we do need the context unpin on retirement and to do the
pin_count check in i915_vma_unbind() after waiting (rather than before,
as we currently do). However, the eviction code will not inspect the
active contexts objects yet (as it will continue to skip over the
ggtt->pin_count on them). The way I allowed ctx objects to be evicted was
to only keep the ctx->state pinned for the duration of the request
construction.

Note that I think it should be a vma->unbind hook not an object level
one (it is i915_vma_unbind, without only a modicum of object level state
being modified in that function).
-Chris



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

Re: [Intel-gfx] [PATCH 1/3] drm/i915: Map the ringbuffer using WB on LLC machines

[Ville Syrjälä]

On Thu, Oct 08, 2015 at 01:39:54PM +0100, Chris Wilson wrote:
> If we have llc coherency, we can write directly into the ringbuffer
> using ordinary cached writes rather than forcing WC access.
> 
> v2: An important consequence is that we can forgo the mappable request
> for WB ringbuffers, allowing for many more simultaneous contexts.

Fits in with my understanding on llc coherency.

Reviewed-by: Ville Syrjälä 

> 
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 70 
> ++---
>  1 file changed, 56 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index d608be46ea6e..f81ec7785fac 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1966,11 +1966,35 @@ static int init_phys_status_page(struct 
> intel_engine_cs *ring)
>  
>  void intel_unpin_ringbuffer_obj(struct intel_ringbuffer *ringbuf)
>  {
> - iounmap(ringbuf->virtual_start);
> + if (HAS_LLC(ringbuf->obj->base.dev) && !ringbuf->obj->stolen)
> + vunmap(ringbuf->virtual_start);
> + else
> + iounmap(ringbuf->virtual_start);
>   ringbuf->virtual_start = NULL;
>   i915_gem_object_ggtt_unpin(ringbuf->obj);
>  }
>  
> +static u32 *vmap_obj(struct drm_i915_gem_object *obj)
> +{
> + struct sg_page_iter sg_iter;
> + struct page **pages;
> + void *addr;
> + int i;
> +
> + pages = drm_malloc_ab(obj->base.size >> PAGE_SHIFT, sizeof(*pages));
> + if (pages == NULL)
> + return NULL;
> +
> + i = 0;
> + for_each_sg_page(obj->pages->sgl, _iter, obj->pages->nents, 0)
> + pages[i++] = sg_page_iter_page(_iter);
> +
> + addr = vmap(pages, i, 0, PAGE_KERNEL);
> + drm_free_large(pages);
> +
> + return addr;
> +}
> +
>  int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev,
>struct intel_ringbuffer *ringbuf)
>  {
> @@ -1978,21 +2002,39 @@ int intel_pin_and_map_ringbuffer_obj(struct 
> drm_device *dev,
>   struct drm_i915_gem_object *obj = ringbuf->obj;
>   int ret;
>  
> - ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, PIN_MAPPABLE);
> - if (ret)
> - return ret;
> + if (HAS_LLC(dev_priv) && !obj->stolen) {
> + ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, 0);
> + if (ret)
> + return ret;
>  
> - ret = i915_gem_object_set_to_gtt_domain(obj, true);
> - if (ret) {
> - i915_gem_object_ggtt_unpin(obj);
> - return ret;
> - }
> + ret = i915_gem_object_set_to_cpu_domain(obj, true);
> + if (ret) {
> + i915_gem_object_ggtt_unpin(obj);
> + return ret;
> + }
> +
> + ringbuf->virtual_start = vmap_obj(obj);
> + if (ringbuf->virtual_start == NULL) {
> + i915_gem_object_ggtt_unpin(obj);
> + return -ENOMEM;
> + }
> + } else {
> + ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, PIN_MAPPABLE);
> + if (ret)
> + return ret;
>  
> - ringbuf->virtual_start = ioremap_wc(dev_priv->gtt.mappable_base +
> - i915_gem_obj_ggtt_offset(obj), ringbuf->size);
> - if (ringbuf->virtual_start == NULL) {
> - i915_gem_object_ggtt_unpin(obj);
> - return -EINVAL;
> + ret = i915_gem_object_set_to_gtt_domain(obj, true);
> + if (ret) {
> + i915_gem_object_ggtt_unpin(obj);
> + return ret;
> + }
> +
> + ringbuf->virtual_start = ioremap_wc(dev_priv->gtt.mappable_base 
> +
> + 
> i915_gem_obj_ggtt_offset(obj), ringbuf->size);
> + if (ringbuf->virtual_start == NULL) {
> + i915_gem_object_ggtt_unpin(obj);
> + return -EINVAL;
> + }
>   }
>  
>   return 0;
> -- 
> 2.6.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH v6] drm/i915: Add soft-pinning API for execbuffer

[Chris Wilson]

On Fri, Oct 16, 2015 at 07:39:20PM +0530, Goel, Akash wrote:
> 
> Discussed sometime back about this patch with Chris. He mainly has 2
> concerns with it.
> 1. The linear walk used by the patch to detect the overlapping
> objects would be expensive.
> 2. Restriction to disallow !RCS submissions for non-default
> contexts, which could lead to lot of conflicts for the placements,
> once multiple engines like media/blit/vebox are used
> 
> Both of them can be addressed by the subsequent patches.
> Chris already has the patch ready to reduce the validation overhead
> with the use of rbtree and there should be no implications of
> allowing the non-default contexts for !RCS submissions in
> execbuffer.
> 
> In the standalone form, the patch looks good, so
> Reviewed-by: "Akash Goel "

I am sorry, but this patch does not reflect the final version that I
wrote, in particular does not provide the support I actually use inside
the kernel.
-Chris

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

[Intel-gfx] [PATCH v6 19/23] drm/i915: BDW: Pipe level Gamma correction

[Shashank Sharma]

BDW/SKL/BXT platforms support various Gamma correction modes
which are:
1. Legacy 8-bit mode
2. 10-bit mode
3. Split mode
4. 12-bit mode

This patch does the following:
1. Adds the core function to program Gamma correction values
   for BDW/SKL/BXT platforms
2. Adds Gamma correction macros/defines

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal Malladi 
---
 drivers/gpu/drm/i915/i915_reg.h|  25 ++-
 drivers/gpu/drm/i915/intel_color_manager.c | 281 +
 drivers/gpu/drm/i915/intel_color_manager.h |   6 +
 3 files changed, 310 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3db42f4..39fbafc 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5685,11 +5685,15 @@ enum skl_disp_power_wells {
 /* legacy palette */
 #define _LGC_PALETTE_A   0x4a000
 #define _LGC_PALETTE_B   0x4a800
-#define LGC_PALETTE(pipe, i) (_PIPE(pipe, _LGC_PALETTE_A, _LGC_PALETTE_B) + 
(i) * 4)
+#define _LGC_PALETTE_C   0x4b000
+#define LGC_PALETTE(pipe, i) (_PIPE3(pipe, _LGC_PALETTE_A, _LGC_PALETTE_B, \
+_LGC_PALETTE_C) + (i) * 4)
 
 #define _GAMMA_MODE_A  0x4a480
 #define _GAMMA_MODE_B  0x4ac80
-#define GAMMA_MODE(pipe) _PIPE(pipe, _GAMMA_MODE_A, _GAMMA_MODE_B)
+#define _GAMMA_MODE_C  0x4b480
+#define GAMMA_MODE(pipe) \
+   _PIPE3(pipe, _GAMMA_MODE_A, _GAMMA_MODE_B, _GAMMA_MODE_C)
 #define GAMMA_MODE_MODE_MASK   (3 << 0)
 #define GAMMA_MODE_MODE_8BIT   (0 << 0)
 #define GAMMA_MODE_MODE_10BIT  (1 << 0)
@@ -8172,6 +8176,23 @@ enum skl_disp_power_wells {
 #define _PIPE_CSC_BASE(pipe) \
(_PIPE3(pipe, PIPEA_CGM_CSC, PIPEB_CGM_CSC, PIPEC_CGM_CSC))
 
+/* BDW gamma correction */
+#define PAL_PREC_INDEX_A   0x4A400
+#define PAL_PREC_INDEX_B   0x4AC00
+#define PAL_PREC_INDEX_C   0x4B400
+#define PAL_PREC_DATA_A0x4A404
+#define PAL_PREC_DATA_B0x4AC04
+#define PAL_PREC_DATA_C0x4B404
+#define PAL_PREC_GCMAX_A   0x4A410
+#define PAL_PREC_GCMAX_B   0x4AC10
+#define PAL_PREC_GCMAX_C   0x4B410
+
+#define _PREC_PAL_INDEX(pipe) \
+   (_PIPE3(pipe, PAL_PREC_INDEX_A, PAL_PREC_INDEX_B, PAL_PREC_INDEX_C))
+#define _PREC_PAL_DATA(pipe) \
+   (_PIPE3(pipe, PAL_PREC_DATA_A, PAL_PREC_DATA_B, PAL_PREC_DATA_C))
+#define _PREC_PAL_GCMAX(pipe) \
+   (_PIPE3(pipe, PAL_PREC_GCMAX_A, PAL_PREC_GCMAX_B, PAL_PREC_GCMAX_C))
 
 
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
b/drivers/gpu/drm/i915/intel_color_manager.c
index 6b59531..44f99be 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -27,6 +27,285 @@
 
 #include "intel_color_manager.h"
 
+static void bdw_write_8bit_gamma_legacy(struct drm_device *dev,
+   struct drm_r32g32b32 *correction_values, u32 palette)
+{
+   u16 blue_fract, green_fract, red_fract;
+   u32 blue, green, red;
+   u32 count = 0;
+   u32 word = 0;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+
+   while (count < BDW_8BIT_GAMMA_MAX_VALS) {
+   blue = correction_values[count].b32;
+   green = correction_values[count].g32;
+   red = correction_values[count].r32;
+
+   /*
+   * Maximum possible gamma correction value supported
+   * for BDW is 0x, so clamp the values accordingly
+   */
+   if (blue >= BDW_MAX_GAMMA)
+   blue = BDW_MAX_GAMMA;
+   if (green >= BDW_MAX_GAMMA)
+   green = BDW_MAX_GAMMA;
+   if (red >= BDW_MAX_GAMMA)
+   red = BDW_MAX_GAMMA;
+
+   blue_fract = GET_BITS(blue, 16, 8);
+   green_fract = GET_BITS(green, 16, 8);
+   red_fract = GET_BITS(red, 16, 8);
+
+   /* Blue (7:0) Green (15:8) and Red (23:16) */
+   SET_BITS(word, blue_fract, 0, 8);
+   SET_BITS(word, green_fract, 8, 8);
+   SET_BITS(word, blue_fract, 16, 8);
+   I915_WRITE(palette, word);
+   palette += 4;
+   count++;
+   }
+}
+
+static void bdw_write_10bit_gamma_precision(struct drm_device *dev,
+   struct drm_r32g32b32 *correction_values, u32 pal_prec_data,
+   u32 no_of_coeff)
+{
+   u16 blue_fract, green_fract, red_fract;
+   u32 word = 0;
+   u32 count = 0;
+   u32 blue, green, red;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+
+   while (count < no_of_coeff) {
+
+   blue = correction_values[count].b32;
+   green = correction_values[count].g32;
+   red = 

[Intel-gfx] [PATCH v6 17/23] drm/i915: Attach color properties to CRTC

[Shashank Sharma]

Function intel_attach_color_properties_to_crtc attaches a
color property to its CRTC object. This patch calls this
function from crtc initialization sequence.

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal Malladi 
---
 drivers/gpu/drm/i915/intel_display.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f94ed6d..61562a3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13861,6 +13861,7 @@ static void intel_crtc_init(struct drm_device *dev, int 
pipe)
intel_crtc->cursor_size = ~0;
 
intel_crtc->wm.cxsr_allowed = true;
+   intel_attach_color_properties_to_crtc(dev, _crtc->base);
 
BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
   dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
-- 
1.9.1

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

[Intel-gfx] [PATCH v6 05/23] drm: Add get property support for color manager

[Shashank Sharma]

As per the DRM get_property implementation for a blob, framework
is supposed to return the blob_id to the caller. All the color
management blobs are saved in CRTC state during the set call.

This patch adds get_property support for color management
properties, by referring to the existing blob for the property
and passing its blob_id.

Signed-off-by: Shashank Sharma 
---
 drivers/gpu/drm/drm_atomic.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 12a34e9..b49aaeb 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -499,6 +499,14 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
*val = state->active;
else if (property == config->prop_mode_id)
*val = (state->mode_blob) ? state->mode_blob->base.id : 0;
+   else if (property == config->cm_palette_after_ctm_property)
+   *val = (state->palette_after_ctm_blob) ?
+   state->palette_after_ctm_blob->base.id : 0;
+   else if (property == config->cm_palette_before_ctm_property)
+   *val = (state->palette_before_ctm_blob) ?
+   state->palette_before_ctm_blob->base.id : 0;
+   else if (property == config->cm_ctm_property)
+   *val = (state->ctm_blob) ? state->ctm_blob->base.id : 0;
else if (crtc->funcs->atomic_get_property)
return crtc->funcs->atomic_get_property(crtc, state, property, 
val);
else
-- 
1.9.1

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

[Intel-gfx] [PATCH v6 14/23] drm/i915: CHV: Pipe level degamma correction

[Shashank Sharma]

CHV/BSW supports Degamma color correction, which linearizes all
the non-linear color values. This will be applied before Color
Transformation.

This patch does the following:
1. Attach deGamma property to CRTC
2. Add the core function to program DeGamma correction values for
   CHV/BSW platform
2. Add DeGamma correction macros/defines

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal Malladi 
---
 drivers/gpu/drm/i915/i915_reg.h|  6 ++
 drivers/gpu/drm/i915/intel_color_manager.c | 92 ++
 drivers/gpu/drm/i915/intel_color_manager.h |  5 ++
 3 files changed, 103 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 45ddd84..1e46562 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8160,4 +8160,10 @@ enum skl_disp_power_wells {
 #define _PIPE_GAMMA_BASE(pipe) \
(_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA))
 
+#define PIPEA_CGM_DEGAMMA  (VLV_DISPLAY_BASE + 0x66000)
+#define PIPEB_CGM_DEGAMMA  (VLV_DISPLAY_BASE + 0x68000)
+#define PIPEC_CGM_DEGAMMA  (VLV_DISPLAY_BASE + 0x6A000)
+#define _PIPE_DEGAMMA_BASE(pipe) \
+   (_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA, PIPEC_CGM_DEGAMMA))
+
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
b/drivers/gpu/drm/i915/intel_color_manager.c
index acb9647..1bbad79 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -27,6 +27,92 @@
 
 #include "intel_color_manager.h"
 
+static int chv_set_degamma(struct drm_device *dev,
+   struct drm_property_blob *blob, struct drm_crtc *crtc)
+{
+   u16 red_fract, green_fract, blue_fract;
+   u32 red, green, blue;
+   u32 num_samples;
+   u32 word = 0;
+   u32 count, cgm_control_reg, cgm_degamma_reg;
+   enum pipe pipe;
+   struct drm_palette *degamma_data;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct drm_r32g32b32 *correction_values = NULL;
+   struct drm_crtc_state *state = crtc->state;
+
+   if (WARN_ON(!blob))
+   return -EINVAL;
+
+   degamma_data = (struct drm_palette *)blob->data;
+   pipe = to_intel_crtc(crtc)->pipe;
+   num_samples = blob->length / sizeof(struct drm_r32g32b32);
+
+   switch (num_samples) {
+   case GAMMA_DISABLE_VALS:
+   /* Disable DeGamma functionality on Pipe - CGM Block */
+   cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
+   cgm_control_reg &= ~CGM_DEGAMMA_EN;
+   state->palette_before_ctm_blob = NULL;
+
+   I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
+   DRM_DEBUG_DRIVER("DeGamma disabled on Pipe %c\n",
+   pipe_name(pipe));
+   break;
+
+   case CHV_DEGAMMA_MAX_VALS:
+   cgm_degamma_reg = _PIPE_DEGAMMA_BASE(pipe);
+   count = 0;
+   correction_values = (struct drm_r32g32b32 *)_data->lut;
+   while (count < CHV_DEGAMMA_MAX_VALS) {
+   blue = correction_values[count].b32;
+   green = correction_values[count].g32;
+   red = correction_values[count].r32;
+
+   if (blue > CHV_MAX_GAMMA)
+   blue = CHV_MAX_GAMMA;
+
+   if (green > CHV_MAX_GAMMA)
+   green = CHV_MAX_GAMMA;
+
+   if (red > CHV_MAX_GAMMA)
+   red = CHV_MAX_GAMMA;
+
+   blue_fract = GET_BITS(blue, 8, 14);
+   green_fract = GET_BITS(green, 8, 14);
+   red_fract = GET_BITS(red, 8, 14);
+
+   /* Green (29:16) and Blue (13:0) in DWORD1 */
+   SET_BITS(word, green_fract, 16, 14);
+   SET_BITS(word, green_fract, 0, 14);
+   I915_WRITE(cgm_degamma_reg, word);
+   cgm_degamma_reg += 4;
+
+   /* Red (13:0) to be written to DWORD2 */
+   word = red_fract;
+   I915_WRITE(cgm_degamma_reg, word);
+   cgm_degamma_reg += 4;
+   count++;
+   }
+
+   DRM_DEBUG_DRIVER("DeGamma LUT loaded for Pipe %c\n",
+   pipe_name(pipe));
+
+   /* Enable DeGamma on Pipe */
+   I915_WRITE(_PIPE_CGM_CONTROL(pipe),
+   I915_READ(_PIPE_CGM_CONTROL(pipe)) | CGM_DEGAMMA_EN);
+
+   DRM_DEBUG_DRIVER("DeGamma correction enabled on Pipe %c\n",
+   pipe_name(pipe));
+   break;
+
+   default:
+   DRM_ERROR("Invalid number of samples for 

[Intel-gfx] [PATCH v6 10/23] drm/i915: Register color correction capabilities

[Shashank Sharma]

From DRM color management:

DRM color manager supports these color properties:
1. "ctm": Color transformation matrix property, where a
   color transformation matrix of 9 correction values gets
   applied as correction.
2. "palette_before_ctm": for corrections which get applied
   beore color transformation matrix correction.
3. "palette_after_ctm": for corrections which get applied
   after color transformation matrix correction.

These color correction capabilities may differ per platform, supporting
various different no. of correction coefficients. So DRM color manager
support few properties using which a user space can query the platform's
capability, and prepare color correction accordingly.
These query properties are:
1. cm_coeff_after_ctm_property
2. cm_coeff_before_ctm_property
  (CTM is fix to 9 coefficients across industry)

Now, Intel color manager registers:
==
1. Gamma correction property as "palette_after_ctm" property
2. Degamma correction capability as "palette_bafore_ctm" property
   capability as "palette_after_ctm" DRM color property hook.
3. CSC as "ctm" property.

So finally, This patch does the following:
1. Add a function which loads the platform's color correction
   capabilities in the cm_crtc_palette_capabilities_property structure.
2. Attaches the cm_crtc_palette_capabilities_property to every CRTC
   getting initiaized.
3. Adds two new parameters "num_samples_after_ctm" and
   "num_samples_before_ctm" in intel_device_info as gamma and
   degamma coefficients vary per platform basis.

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal Malladi 
---
 drivers/gpu/drm/i915/i915_drv.h|  2 ++
 drivers/gpu/drm/i915/intel_color_manager.c | 31 ++
 2 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8afda45..613bee2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -789,6 +789,8 @@ struct intel_device_info {
u8 num_sprites[I915_MAX_PIPES];
u8 gen;
u8 ring_mask; /* Rings supported by the HW */
+   u16 num_samples_after_ctm;
+   u16 num_samples_before_ctm;
DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG, SEP_SEMICOLON);
/* Register offsets for the various display pipes and transcoders */
int pipe_offsets[I915_MAX_TRANSCODERS];
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
b/drivers/gpu/drm/i915/intel_color_manager.c
index b03ee94..334bfff 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -30,4 +30,35 @@
 void intel_attach_color_properties_to_crtc(struct drm_device *dev,
struct drm_crtc *crtc)
 {
+   struct drm_mode_config *config = >mode_config;
+   struct drm_mode_object *mode_obj = >base;
+
+   /*
+   * Register:
+   * =
+   * Gamma correction as palette_after_ctm property
+   * Degamma correction as palette_before_ctm property
+   *
+   * Load:
+   * =
+   * no. of coefficients supported on this platform for gamma
+   * and degamma with the query properties. A user
+   * space agent should read these query property, and prepare
+   * the color correction values accordingly. Its expected from the
+   * driver to load the right number of coefficients during the init
+   * phase.
+   */
+   if (config->cm_coeff_after_ctm_property) {
+   drm_object_attach_property(mode_obj,
+   config->cm_coeff_after_ctm_property,
+   INTEL_INFO(dev)->num_samples_after_ctm);
+   DRM_DEBUG_DRIVER("Gamma query property initialized\n");
+   }
+
+   if (config->cm_coeff_before_ctm_property) {
+   drm_object_attach_property(mode_obj,
+   config->cm_coeff_before_ctm_property,
+   INTEL_INFO(dev)->num_samples_before_ctm);
+   DRM_DEBUG_DRIVER("Degamma query property initialized\n");
+   }
 }
-- 
1.9.1

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

[Intel-gfx] [PATCH v6 16/23] drm/i915: Commit color correction to CRTC

[Shashank Sharma]

The color correction blob values are loaded during set_property
calls. This patch adds a function to find the blob and apply the
correction values to the display registers, during the atomic
commit call.

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal Malladi 
---
 drivers/gpu/drm/i915/intel_color_manager.c | 44 ++
 drivers/gpu/drm/i915/intel_display.c   |  2 ++
 drivers/gpu/drm/i915/intel_drv.h   |  2 ++
 3 files changed, 48 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
b/drivers/gpu/drm/i915/intel_color_manager.c
index 6661c53..6b59531 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -291,6 +291,50 @@ static int chv_set_gamma(struct drm_device *dev, struct 
drm_property_blob *blob,
}
 }
 
+void intel_color_manager_crtc_commit(struct drm_device *dev,
+   struct drm_crtc_state *crtc_state)
+{
+   struct drm_property_blob *blob;
+   struct drm_crtc *crtc = crtc_state->crtc;
+   int ret = -EINVAL;
+
+   blob = crtc_state->palette_after_ctm_blob;
+   if (blob) {
+   /* Gamma correction is platform specific */
+   if (IS_CHERRYVIEW(dev))
+   ret = chv_set_gamma(dev, blob, crtc);
+
+   if (ret)
+   DRM_ERROR("set Gamma correction failed\n");
+   else
+   DRM_DEBUG_DRIVER("Gamma correction success\n");
+   }
+
+   blob = crtc_state->palette_before_ctm_blob;
+   if (blob) {
+   /* Degamma correction */
+   if (IS_CHERRYVIEW(dev))
+   ret = chv_set_degamma(dev, blob, crtc);
+
+   if (ret)
+   DRM_ERROR("set degamma correction failed\n");
+   else
+   DRM_DEBUG_DRIVER("degamma correction success\n");
+   }
+
+   blob = crtc_state->ctm_blob;
+   if (blob) {
+   /* CSC correction */
+   if (IS_CHERRYVIEW(dev))
+   ret = chv_set_csc(dev, blob, crtc);
+
+   if (ret)
+   DRM_ERROR("set CSC correction failed\n");
+   else
+   DRM_DEBUG_DRIVER("CSC correction success\n");
+   }
+}
+
 void intel_attach_color_properties_to_crtc(struct drm_device *dev,
struct drm_crtc *crtc)
 {
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 7cad341..f94ed6d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13530,6 +13530,8 @@ static void intel_begin_crtc_commit(struct drm_crtc 
*crtc,
intel_update_pipe_config(intel_crtc, old_intel_state);
else if (INTEL_INFO(dev)->gen >= 9)
skl_detach_scalers(intel_crtc);
+
+   intel_color_manager_crtc_commit(dev, crtc->state);
 }
 
 static void intel_finish_crtc_commit(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2e4e97d..4b24496 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1480,4 +1480,6 @@ extern const struct drm_plane_helper_funcs 
intel_plane_helper_funcs;
 /* intel_color_manager.c */
 void intel_attach_color_properties_to_crtc(struct drm_device *dev,
struct drm_crtc *crtc);
+void intel_color_manager_crtc_commit(struct drm_device *dev,
+   struct drm_crtc_state *crtc_state);
 #endif /* __INTEL_DRV_H__ */
-- 
1.9.1

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

[Intel-gfx] [PATCH v6 15/23] drm/i915: CHV: Pipe level CSC correction

[Shashank Sharma]

CHV/BSW supports Color Space Conversion (CSC) using a 3x3 matrix
that needs to be programmed into CGM (Color Gamut Mapping) registers.

This patch does the following:
1. Attaches CSC property to CRTC
2. Adds the core function to program CSC correction values
3. Adds CSC correction macros

Signed-off-by: Shashank Sharma 
Signed-off-by: Kausal Malladi 
Signed-off-by: Kumar, Kiran S 
---
 drivers/gpu/drm/i915/i915_reg.h|  8 +++
 drivers/gpu/drm/i915/intel_color_manager.c | 99 ++
 drivers/gpu/drm/i915/intel_color_manager.h | 19 ++
 3 files changed, 126 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1e46562..3db42f4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8166,4 +8166,12 @@ enum skl_disp_power_wells {
 #define _PIPE_DEGAMMA_BASE(pipe) \
(_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA, PIPEC_CGM_DEGAMMA))
 
+#define PIPEA_CGM_CSC  (VLV_DISPLAY_BASE + 0x67900)
+#define PIPEB_CGM_CSC  (VLV_DISPLAY_BASE + 0x69900)
+#define PIPEC_CGM_CSC  (VLV_DISPLAY_BASE + 0x6B900)
+#define _PIPE_CSC_BASE(pipe) \
+   (_PIPE3(pipe, PIPEA_CGM_CSC, PIPEB_CGM_CSC, PIPEC_CGM_CSC))
+
+
+
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
b/drivers/gpu/drm/i915/intel_color_manager.c
index 1bbad79..6661c53 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -27,6 +27,98 @@
 
 #include "intel_color_manager.h"
 
+static s32 chv_prepare_csc_coeff(s64 csc_value)
+{
+   s32 csc_int_value;
+   u32 csc_fract_value;
+   s32 csc_s3_12_format;
+
+   if (csc_value >= 0) {
+   csc_value += CHV_CSC_FRACT_ROUNDOFF;
+   if (csc_value > CHV_CSC_COEFF_MAX)
+   csc_value = CHV_CSC_COEFF_MAX;
+   } else {
+   csc_value = -csc_value;
+   csc_value += CHV_CSC_FRACT_ROUNDOFF;
+   if (csc_value > CHV_CSC_COEFF_MAX + 1)
+   csc_value = CHV_CSC_COEFF_MAX + 1;
+   csc_value = -csc_value;
+   }
+
+   csc_int_value = csc_value >> CHV_CSC_COEFF_SHIFT;
+   csc_int_value <<= CHV_CSC_COEFF_INT_SHIFT;
+   if (csc_value < 0)
+   csc_int_value |= CSC_COEFF_SIGN;
+
+   csc_fract_value = csc_value;
+   csc_fract_value >>= CHV_CSC_COEFF_FRACT_SHIFT;
+   csc_s3_12_format = csc_int_value | csc_fract_value;
+
+   return csc_s3_12_format;
+}
+
+static int chv_set_csc(struct drm_device *dev, struct drm_property_blob *blob,
+   struct drm_crtc *crtc)
+{
+   struct drm_ctm *csc_data;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   u32 reg;
+   enum pipe pipe;
+   s32 word = 0, temp;
+   int count = 0;
+
+   if (WARN_ON(!blob))
+   return -EINVAL;
+
+   if (blob->length != sizeof(struct drm_ctm)) {
+   DRM_ERROR("Invalid length of data received\n");
+   return -EINVAL;
+   }
+
+   csc_data = (struct drm_ctm *)blob->data;
+   pipe = to_intel_crtc(crtc)->pipe;
+
+   /* Disable CSC functionality */
+   reg = _PIPE_CGM_CONTROL(pipe);
+   I915_WRITE(reg, I915_READ(reg) & (~CGM_CSC_EN));
+
+   DRM_DEBUG_DRIVER("Disabled CSC Functionality on Pipe %c\n",
+   pipe_name(pipe));
+
+   reg = _PIPE_CSC_BASE(pipe);
+
+   /*
+   * First 8 of 9 CSC correction values go in pair, to first
+   * 4 CSC register (bit 0:15 and 16:31)
+   */
+   while (count < CSC_MAX_VALS - 1) {
+   temp = chv_prepare_csc_coeff(
+   csc_data->ctm_coeff[count]);
+   SET_BITS(word, GET_BITS(temp, 16, 16), 0, 16);
+   count++;
+
+   temp = chv_prepare_csc_coeff(
+   csc_data->ctm_coeff[count]);
+   SET_BITS(word, GET_BITS(temp, 16, 16), 16, 16);
+   count++;
+
+   I915_WRITE(reg, word);
+   reg += 4;
+   }
+
+   /* 9th coeff goes to 5th register, bit 0:16 */
+   temp = chv_prepare_csc_coeff(
+   csc_data->ctm_coeff[count]);
+   SET_BITS(word, GET_BITS(temp, 16, 16), 0, 16);
+   I915_WRITE(reg, word);
+
+   /* Enable CSC functionality */
+   reg = _PIPE_CGM_CONTROL(pipe);
+   I915_WRITE(reg, I915_READ(reg) | CGM_CSC_EN);
+   DRM_DEBUG_DRIVER("CSC enabled on Pipe %c\n", pipe_name(pipe));
+   return 0;
+}
+
 static int chv_set_degamma(struct drm_device *dev,
struct drm_property_blob *blob, struct drm_crtc *crtc)
 {
@@ -247,4 +339,11 @@ void intel_attach_color_properties_to_crtc(struct 
drm_device *dev,
config->cm_palette_before_ctm_property, 0);
  

Re: [Intel-gfx] [PATCH v2] drm/i915/hsw: keep gamma and CSC enabled for primary plane disable

[Bob Paauwe]

On Thu, 15 Oct 2015 15:41:30 +0300
Ville Syrjälä  wrote:

> On Thu, Oct 15, 2015 at 02:31:09PM +0200, Daniel Vetter wrote:
> > 
> > On Thu, Oct 15, 2015 at 12:51 AM, Kevin Strasser 
> >  wrote:
> > > On HSW the crc differs between black and disabled primary planes, causing 
> > > an
> > > assert to fail in the kms_universal_plane test. It seems that gamma 
> > > correction
> > > and color space conversion are causing the black primary plane case to 
> > > result in
> > > a brighter color than the disabled primary plane case.
> > >
> > > Keep gamma and CSC bits enabled for plane disable path on HSW.
> > >
> > > v2: Avoid use of RMW
> > > Keep path unchanged for non-HSW users
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89331
> > > Testcase: igt/kms_universal_plane/universal-plane-pipe-A-functional
> > > Signed-off-by: Kevin Strasser 
> > 
> > With big discussions please add everyone who participated (Ville, Chris,
> > Jani, me) to the cc list of the sob section of the patch when doing a new
> > revisions.
> > 
> > Bob did something eerily similarly a while ago to fix crc failures:
> > 
> > http://lists.freedesktop.org/archives/intel-gfx/2015-August/074657.html
> > http://lists.freedesktop.org/archives/intel-gfx/2015-August/074828.html
> > 
> > Unfortunately those patches did go nowhere :( Related?
> 
> Those seem to be for active plane cases. I'll go review them...
> 

I've been looking at the same test case failure on BXT. The bxt/skl
update primary plane function is similar to HSW's in that it was
clearing the gamma and csc bits when the plane was disabled. So I first
tried the same fix but it didn't work.  Matt and I were discussing this
and thought it might be the background (PIPE_BOTTOM_COLOR) that was
causing the CRC failures.  And yes, it is.  The PIPE_BOTTOM_COLOR also
has gamma and csc enable bits which default to off.  If those are
flipped on, then the CRC's match between a disabled plane and a black
plane. 

So it seems to make CRC's match we need to make sure that all planes
(primary/sprite/cursor/bottom) have the same gamma/csc settings.

Chandra had a patch that adds a property for bottom color a while back
but it seems to have stalled. 

http://lists.freedesktop.org/archives/intel-gfx/2015-June/069836.html

-- 
--
Bob Paauwe  
bob.j.paa...@intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193

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

[Intel-gfx] [PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier

[Vivek Kasireddy]

The main goal of this subtest is to verify whether flipping a
framebuffer with a Y fb modifier (90/270 degree rotation) and
an associated Y-tiled object works or not.

Cc: Tvrtko Ursulin 
Signed-off-by: Vivek Kasireddy 
---
 tests/kms_rotation_crc.c | 83 
 1 file changed, 83 insertions(+)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index cc9847e..bb9aecb 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -264,6 +264,83 @@ static void test_plane_rotation(data_t *data, enum 
igt_plane plane_type)
igt_require_f(valid_tests, "no valid crtc/connector combinations 
found\n");
 }
 
+static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane 
plane_type)
+{
+   igt_display_t *display = >display;
+   uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
+   uint32_t format = DRM_FORMAT_XRGB;
+   int bpp = igt_drm_format_to_bpp(format);
+   enum igt_commit_style commit = COMMIT_LEGACY;
+   int fd = data->gfx_fd;
+   int valid_tests = 0;
+   igt_output_t *output;
+   int ret;
+
+   if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
+   igt_require(data->display.has_universal_planes);
+   commit = COMMIT_UNIVERSAL;
+   }
+
+   for_each_connected_output(display, output) {
+   igt_plane_t *plane;
+   drmModeModeInfo *mode = igt_output_get_mode(output);
+   unsigned int w = mode->hdisplay;
+   unsigned int h = mode->vdisplay;
+   unsigned int stride, size;
+   uint32_t gem_handle;
+
+   for (stride = 512; stride < (w * bpp / 8); stride *= 2)
+   ;
+   for (size = 1024*1024; size < stride * h; size *= 2)
+   ;
+
+   gem_handle = gem_create(fd, size);
+   ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
+   igt_assert(ret == 0);
+
+   do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
+ format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
+ >fb.fb_id));
+
+   data->fb.width = w;
+   data->fb.height = h;
+   data->fb.gem_handle = gem_handle;
+   data->fb.stride = stride;
+   data->fb.size = size;
+   data->fb.tiling = tiling;
+   data->fb.drm_format = format;
+
+   plane = igt_output_get_plane(output, plane_type);
+   igt_require(igt_plane_supports_rotation(plane));
+
+   igt_plane_set_fb(plane, NULL);
+   igt_display_commit(display);
+
+   igt_plane_set_rotation(plane, data->rotation);
+   paint_squares(data, mode, IGT_ROTATION_0, plane);
+   igt_plane_set_fb(plane, >fb);
+
+   drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
+DRM_MODE_OBJECT_PLANE,
+plane->rotation_property,
+plane->rotation);
+   ret = igt_display_try_commit2(display, commit);
+   igt_assert(ret == 0);
+
+   kmstest_restore_vt_mode();
+   kmstest_set_vt_graphics_mode();
+   igt_display_commit2(display, commit);
+
+   valid_tests++;
+   igt_remove_fb(fd, >fb);
+   igt_output_set_pipe(output, PIPE_ANY);
+
+   igt_plane_set_fb(plane, NULL);
+   igt_display_commit(display);
+   }
+   igt_require_f(valid_tests, "no valid crtc/connector combinations 
found\n");
+}
+
 igt_main
 {
data_t data = {};
@@ -345,6 +422,12 @@ igt_main
test_plane_rotation(, IGT_PLANE_PRIMARY);
}
 
+   igt_subtest_f("primary-rotation-90-Y-tiled") {
+   igt_require(gen >= 9);
+   data.rotation = IGT_ROTATION_90;
+   test_plane_rotation_ytiled_obj(, IGT_PLANE_PRIMARY);
+   }
+
igt_fixture {
igt_display_fini();
}
-- 
2.4.3

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

[Intel-gfx] [PATCH 2/2] drm/fb-helper: Fix fb refcounting in pan_display_atomic

[Daniel Vetter]

In

commit bbb1e52402b2a288b09ae37e8182599931c7e9df
Author: Rob Clark 
Date:   Tue Aug 25 15:35:58 2015 -0400

drm/fb-helper: atomic restore_fbdev_mode()..

we've forgotten to do the plane->old_fb refcount dance for
pan_display_atomic, which can result in refcount leaks if the current
configuration is not from fbcon. Which apparently can happen when
vt-switching - fbcon does a pan first before a set_par.

OCD-align function parameters while at it.

Cc: Rob Clark 
Cc: Rodrigo Vivi 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92483
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_fb_helper.c | 27 +--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 0ac0fcc9b0d2..b2cf28dd90fe 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1249,6 +1249,8 @@ retry:
 
mode_set = _helper->crtc_info[i].mode_set;
 
+   mode_set->crtc->primary->old_fb = mode_set->crtc->primary->fb;
+
mode_set->x = var->xoffset;
mode_set->y = var->yoffset;
 
@@ -1264,13 +1266,34 @@ retry:
info->var.xoffset = var->xoffset;
info->var.yoffset = var->yoffset;
 
-   return 0;
 
 fail:
+   for(i = 0; i < fb_helper->crtc_count; i++) {
+   struct drm_mode_set *mode_set;
+   struct drm_plane *plane;
+
+   mode_set = _helper->crtc_info[i].mode_set;
+   plane = mode_set->crtc->primary;
+
+   if (ret == 0) {
+   struct drm_framebuffer *new_fb = plane->state->fb;
+
+   if (new_fb)
+   drm_framebuffer_reference(new_fb);
+   plane->fb = new_fb;
+   plane->crtc = plane->state->crtc;
+
+   if (plane->old_fb)
+   drm_framebuffer_unreference(plane->old_fb);
+   }
+   plane->old_fb = NULL;
+   }
+
if (ret == -EDEADLK)
goto backoff;
 
-   drm_atomic_state_free(state);
+   if (ret != 0)
+   drm_atomic_state_free(state);
 
return ret;
 
-- 
2.5.1

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

[Intel-gfx] [PATCH 1/2] drm/fb-helper: Set plane rotation directly

[Daniel Vetter]

The point behind standardizing properties into core drm state
structures is also that internal code looks prettiers. Take advantage
of that and set rotation directly in the fbdev atomic code.

Cc: Rob Clark 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_fb_helper.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index bd6d4ab27512..0ac0fcc9b0d2 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -360,11 +360,7 @@ retry:
goto fail;
}
 
-   ret = drm_atomic_plane_set_property(plane, plane_state,
-   dev->mode_config.rotation_property,
-   BIT(DRM_ROTATE_0));
-   if (ret != 0)
-   goto fail;
+   plane_state->rotation = BIT(DRM_ROTATE_0);
 
/* disable non-primary: */
if (plane->type == DRM_PLANE_TYPE_PRIMARY)
-- 
2.5.1

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

Re: [Intel-gfx] [PATCH] drm/i915: Report context GTT size

[Tvrtko Ursulin]

On 14/10/15 14:17, Chris Wilson wrote:

Since the beginning we have conflated the size of the global GTT with
that of the per-process context sizes. In recent times (gen8+), those
are no longer the same where the global GTT is limited to 2/4GiB but the
per-process GTT may be anything up to 256TiB. Userspace knows nothing of
this discrepancy and outside of one or two hacks, uses the getaperture
ioctl to determine the maximum size it can use. Let's leave that as
reporting the global GTT and use the context reporting method to
describe the per-process value (which naturally fallsback to reporting
the aliasing or global on older platforms, so userspace can always use
this method where available).

Testcase: igt/gem_userptr_blits/minor-normal-sync
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90065
Signed-off-by: Chris Wilson 
---
  drivers/gpu/drm/i915/i915_gem_context.c | 8 
  include/uapi/drm/i915_drm.h | 5 +++--
  2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 339a9d628f1e..cecb156c6f76 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -1087,6 +1087,14 @@ int i915_gem_context_getparam_ioctl(struct drm_device 
*dev, void *data,
case I915_CONTEXT_PARAM_NO_ZEROMAP:
args->value = ctx->flags & CONTEXT_NO_ZEROMAP;
break;
+   case I915_CONTEXT_PARAM_GTT_SIZE:
+   if (ctx->ppgtt)
+   args->value = ctx->ppgtt->base.total;
+   else if (to_i915(dev)->mm.aliasing_ppgtt)
+   args->value = 
to_i915(dev)->mm.aliasing_ppgtt->base.total;
+   else
+   args->value = to_i915(dev)->gtt.base.total;
+   break;
default:
ret = -EINVAL;
break;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 83f60f01dca2..3f334967aa1b 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1130,8 +1130,9 @@ struct drm_i915_gem_context_param {
__u32 ctx_id;
__u32 size;
__u64 param;
-#define I915_CONTEXT_PARAM_BAN_PERIOD 0x1
-#define I915_CONTEXT_PARAM_NO_ZEROMAP 0x2
+#define I915_CONTEXT_PARAM_BAN_PERIOD  0x1
+#define I915_CONTEXT_PARAM_NO_ZEROMAP  0x2
+#define I915_CONTEXT_PARAM_GTT_SIZE0x3
__u64 value;
  };


Implementation looks fine,
Reviewed-by: Tvrtko Ursulin 

I have a slight unknown relating to how long would this ABI be useful. 
If things are moving towards SVM, and the fact pre-gen8 platforms can 
already use get_aperture, would that make it a bit short lived?


And get_aperture would even be a better place for this if PPGTT size 
will always be the same for all clients.


Only if we consider that one day we might want to regulate available 
address space available to clients (so implement also a setter for the 
context param), then per-ctx makes sense. (If again, SVM does not make 
this irrelevant.)


This address space limiting kind of sounds interesting, together with 
the incoming GPU scheduling priorities, but then for the both I am not 
sure if an appropriate mechanism could be constructed to use it in a 
classical Unix sense, where you could set limits and inherit them from 
parent to child. To construct some sort of a launcher with lower 
priority / memory use.


Regards,

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

Re: [Intel-gfx] [PATCH 2/2] drm/fb-helper: Fix fb refcounting in pan_display_atomic

[Rob Clark]

On Fri, Oct 16, 2015 at 12:23 PM, Daniel Vetter  wrote:
> In
>
> commit bbb1e52402b2a288b09ae37e8182599931c7e9df
> Author: Rob Clark 
> Date:   Tue Aug 25 15:35:58 2015 -0400
>
> drm/fb-helper: atomic restore_fbdev_mode()..
>
> we've forgotten to do the plane->old_fb refcount dance for
> pan_display_atomic, which can result in refcount leaks if the current
> configuration is not from fbcon. Which apparently can happen when
> vt-switching - fbcon does a pan first before a set_par.

oh, whoops

> OCD-align function parameters while at it.

did you mean to drop that line from the commit msg, or include an
extra hunk that you missed?

at any rate,

Reviewed-by: Rob Clark 

> Cc: Rob Clark 
> Cc: Rodrigo Vivi 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92483
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 27 +--
>  1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 0ac0fcc9b0d2..b2cf28dd90fe 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1249,6 +1249,8 @@ retry:
>
> mode_set = _helper->crtc_info[i].mode_set;
>
> +   mode_set->crtc->primary->old_fb = mode_set->crtc->primary->fb;
> +
> mode_set->x = var->xoffset;
> mode_set->y = var->yoffset;
>
> @@ -1264,13 +1266,34 @@ retry:
> info->var.xoffset = var->xoffset;
> info->var.yoffset = var->yoffset;
>
> -   return 0;
>
>  fail:
> +   for(i = 0; i < fb_helper->crtc_count; i++) {
> +   struct drm_mode_set *mode_set;
> +   struct drm_plane *plane;
> +
> +   mode_set = _helper->crtc_info[i].mode_set;
> +   plane = mode_set->crtc->primary;
> +
> +   if (ret == 0) {
> +   struct drm_framebuffer *new_fb = plane->state->fb;
> +
> +   if (new_fb)
> +   drm_framebuffer_reference(new_fb);
> +   plane->fb = new_fb;
> +   plane->crtc = plane->state->crtc;
> +
> +   if (plane->old_fb)
> +   drm_framebuffer_unreference(plane->old_fb);
> +   }
> +   plane->old_fb = NULL;
> +   }
> +
> if (ret == -EDEADLK)
> goto backoff;
>
> -   drm_atomic_state_free(state);
> +   if (ret != 0)
> +   drm_atomic_state_free(state);
>
> return ret;
>
> --
> 2.5.1
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 5/5] drm: Check plane src coordinates correctly during page flip for atomic drivers

[Tvrtko Ursulin]

On 16/10/15 17:27, Matt Roper wrote:

On Thu, Oct 15, 2015 at 08:40:02PM +0300, ville.syrj...@linux.intel.com wrote:

From: Ville Syrjälä 

Instead of relying on the old crtc-{x,y,mode} gunk, dig out the primary
plane coordinates from the plane state when checking them against the
new framebuffer during page flip.

Cc: Matt Roper 
Cc: Tvrtko Ursulin 
Cc: Daniel Vetter 
Signed-off-by: Ville Syrjälä 


For the series:

Reviewed-by: Matt Roper 

I also confirmed that the i-g-t test I wrote here:
http://lists.freedesktop.org/archives/intel-gfx/2015-October/077394.html
now passes with your patch series, so I believe Tvrtko's original bug
report should be fixed.


Oh I did not realize this series is about this, perhaps because I did 
not see 1/5 which maybe had some more obvious clues. :)


Great, that means if we decide to merge "drm/i915: Consider plane 
rotation when calculating stride in skl_do_mmio_flip" and 
"kms_rotation_crc: Exercise page flips with 90 degree rotation" we would 
have working rotated legacy page flip with a test case.


Regards,

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

Re: [Intel-gfx] [PATCH 2/2] drm/fb-helper: Fix fb refcounting in pan_display_atomic

[Rodrigo Vivi]

Thanks!

Tested-by: Rodrigo Vivi 


On Fri, Oct 16, 2015 at 9:35 AM Rob Clark  wrote:

> On Fri, Oct 16, 2015 at 12:23 PM, Daniel Vetter 
> wrote:
> > In
> >
> > commit bbb1e52402b2a288b09ae37e8182599931c7e9df
> > Author: Rob Clark 
> > Date:   Tue Aug 25 15:35:58 2015 -0400
> >
> > drm/fb-helper: atomic restore_fbdev_mode()..
> >
> > we've forgotten to do the plane->old_fb refcount dance for
> > pan_display_atomic, which can result in refcount leaks if the current
> > configuration is not from fbcon. Which apparently can happen when
> > vt-switching - fbcon does a pan first before a set_par.
>
> oh, whoops
>
> > OCD-align function parameters while at it.
>
> did you mean to drop that line from the commit msg, or include an
> extra hunk that you missed?
>
> at any rate,
>
> Reviewed-by: Rob Clark 
>
> > Cc: Rob Clark 
> > Cc: Rodrigo Vivi 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92483
> > Signed-off-by: Daniel Vetter 
> > ---
> >  drivers/gpu/drm/drm_fb_helper.c | 27 +--
> >  1 file changed, 25 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> b/drivers/gpu/drm/drm_fb_helper.c
> > index 0ac0fcc9b0d2..b2cf28dd90fe 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -1249,6 +1249,8 @@ retry:
> >
> > mode_set = _helper->crtc_info[i].mode_set;
> >
> > +   mode_set->crtc->primary->old_fb =
> mode_set->crtc->primary->fb;
> > +
> > mode_set->x = var->xoffset;
> > mode_set->y = var->yoffset;
> >
> > @@ -1264,13 +1266,34 @@ retry:
> > info->var.xoffset = var->xoffset;
> > info->var.yoffset = var->yoffset;
> >
> > -   return 0;
> >
> >  fail:
> > +   for(i = 0; i < fb_helper->crtc_count; i++) {
> > +   struct drm_mode_set *mode_set;
> > +   struct drm_plane *plane;
> > +
> > +   mode_set = _helper->crtc_info[i].mode_set;
> > +   plane = mode_set->crtc->primary;
> > +
> > +   if (ret == 0) {
> > +   struct drm_framebuffer *new_fb =
> plane->state->fb;
> > +
> > +   if (new_fb)
> > +   drm_framebuffer_reference(new_fb);
> > +   plane->fb = new_fb;
> > +   plane->crtc = plane->state->crtc;
> > +
> > +   if (plane->old_fb)
> > +
>  drm_framebuffer_unreference(plane->old_fb);
> > +   }
> > +   plane->old_fb = NULL;
> > +   }
> > +
> > if (ret == -EDEADLK)
> > goto backoff;
> >
> > -   drm_atomic_state_free(state);
> > +   if (ret != 0)
> > +   drm_atomic_state_free(state);
> >
> > return ret;
> >
> > --
> > 2.5.1
> >
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/fb-helper: Fix fb refcounting in pan_display_atomic

[Ville Syrjälä]

On Fri, Oct 16, 2015 at 06:23:14PM +0200, Daniel Vetter wrote:
> In
> 
> commit bbb1e52402b2a288b09ae37e8182599931c7e9df
> Author: Rob Clark 
> Date:   Tue Aug 25 15:35:58 2015 -0400
> 
> drm/fb-helper: atomic restore_fbdev_mode()..
> 
> we've forgotten to do the plane->old_fb refcount dance for
> pan_display_atomic, which can result in refcount leaks if the current
> configuration is not from fbcon. Which apparently can happen when
> vt-switching - fbcon does a pan first before a set_par.
> 
> OCD-align function parameters while at it.
> 
> Cc: Rob Clark 
> Cc: Rodrigo Vivi 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92483
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 27 +--
>  1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 0ac0fcc9b0d2..b2cf28dd90fe 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1249,6 +1249,8 @@ retry:
>  
>   mode_set = _helper->crtc_info[i].mode_set;
>  
> + mode_set->crtc->primary->old_fb = mode_set->crtc->primary->fb;
> +
>   mode_set->x = var->xoffset;
>   mode_set->y = var->yoffset;
>  
> @@ -1264,13 +1266,34 @@ retry:
>   info->var.xoffset = var->xoffset;
>   info->var.yoffset = var->yoffset;
>  
> - return 0;
>  
>  fail:

Time to rename the label too?

> + for(i = 0; i < fb_helper->crtc_count; i++) {
> + struct drm_mode_set *mode_set;
> + struct drm_plane *plane;
> +
> + mode_set = _helper->crtc_info[i].mode_set;
> + plane = mode_set->crtc->primary;
> +
> + if (ret == 0) {
> + struct drm_framebuffer *new_fb = plane->state->fb;
> +
> + if (new_fb)
> + drm_framebuffer_reference(new_fb);
> + plane->fb = new_fb;
> + plane->crtc = plane->state->crtc;
> +
> + if (plane->old_fb)
> + drm_framebuffer_unreference(plane->old_fb);
> + }
> + plane->old_fb = NULL;
> + }
> +
>   if (ret == -EDEADLK)
>   goto backoff;
>  
> - drm_atomic_state_free(state);
> + if (ret != 0)
> + drm_atomic_state_free(state);
>  
>   return ret;
>  
> -- 
> 2.5.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH] drm/i915/skl: Correct other-pipe watermark update condition check

[Ville Syrjälä]

On Mon, Sep 21, 2015 at 11:32:44PM +0530, Kumar, Mahesh wrote:
> If ddb allocation for planes in current CRTC is changed, that doesn't
> lead to ddb allocation change for other CRTCs, because our DDB allocation
> is not dynamic according to plane parameters, ddb is allocated according
> to number of CRTC enabled, & divided equally among CTRC's.
> 
> In current condition check during Watermark calculation, if number of
> plane/ddb allocation changes for current CRTC, Watermark for other pipes
> are recalculated. But there is no change in DDB allocation of other pipe
> so watermark is also not changed, This leads to warning messages.
> WARN_ON(!wm_changed)
> 
> This patch corrects this and check if DDB allocation for pipes is changed,
> then only recalculate watermarks.
> 
> Signed-off-by: Kumar, Mahesh 

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 12 +---
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 62de97e..a1ed920 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3127,14 +3127,12 @@ static bool skl_ddb_allocation_changed(const struct 
> skl_ddb_allocation *new_ddb,
>   struct drm_device *dev = intel_crtc->base.dev;
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   const struct skl_ddb_allocation *cur_ddb = _priv->wm.skl_hw.ddb;
> - enum pipe pipe = intel_crtc->pipe;
> -
> - if (memcmp(new_ddb->plane[pipe], cur_ddb->plane[pipe],
> -sizeof(new_ddb->plane[pipe])))
> - return true;
>  
> - if (memcmp(_ddb->cursor[pipe], _ddb->cursor[pipe],
> - sizeof(new_ddb->cursor[pipe])))
> + /*
> +  * If ddb allocation of pipes chenged, it may require recalculation of
> +  * watermarks
> +  */
> + if (memcmp(new_ddb->pipe, cur_ddb->pipe, sizeof(new_ddb->pipe)))
>   return true;

Hmm. Yeah we start from the previously calculated values for all pioes,
and just rewrite the entry for the current pipe, so doing the memset()
over the whole array is fine.

This function looks to have been this way from the start, and supposedly
I even reviewed it. Oh well, can't catch everything I guess.

Reviewed-by: Ville Syrjälä 

>   return false;
> -- 
> 1.9.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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