Re: [Intel-gfx] [PATCH 13/15] drm/i915: Allow execbuffer to use the first object as the batch

[Kenneth Graunke]

On Friday, July 7, 2017 3:17:22 AM PDT Daniel Vetter wrote:
> On Fri, Mar 17, 2017 at 12:15 PM, Joonas Lahtinen
>  wrote:
> > On to, 2017-03-16 at 13:20 +, Chris Wilson wrote:
> >> Currently, the last object in the execlist is the always the batch.
> >> However, when building the batch buffer we often know the batch object
> >> first and if we can use the first slot in the execlist we can emit
> >> relocation instructions relative to it immediately and avoid a separate
> >> pass to adjust the relocations to point to the last execlist slot.
> >>
> >> Signed-off-by: Chris Wilson 
> >
> > Reviewed-by: Joonas Lahtinen 
> 
> This patch was reviewed/pushed full month before the mesa patch was
> fully reviewed and ready for merging. That's not how uapi is done.
> 
> I've fixed this up now by at least reviewing the mesa patch, but for
> next time around: If you review uapi, and you don't make sure the
> userspace side is in good shape too, then you've not reviewed the
> patch properly.
> 
> Same goes for reviewing and not making sure there's tests, but that's
> another rant.
> 
> Ken, pls make sure we don't end up with another case like resource
> streamer (or tell me I should revert this).
> -Daniel

Sorry, that's partly my bad - I had mentioned to Chris that I wanted this
feature, and planned to use it, but then got distracted with other work and
didn't get around to actually shipping a patch to do so.  Both Chris and I
wrote patches, and IIRC I was benchmarking things when I got distracted.

Basically, I915_EXEC_HANDLE_LUT appeared to be a performance loss in the
CPU bound benchmarks I looked at, because we had to walk over one of the
lists and patch up references to the batch (-1 => actual index).

BATCH_FIRST eliminates that overhead, making HANDLE_LUT actually useful
(although only a small amount).

--Ken

signature.asc
Description: This is a digitally signed message part.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Implement I915_PERF_ADD/REMOVE_CONFIG interface

[Matthew Auld]

On 7 July 2017 at 18:08, Lionel Landwerlin
 wrote:
> From: Matthew Auld 
>
> The motivation behind this new interface is expose at runtime the
> creation of new OA configs which can be used as part of the i915 perf
> open interface. This will enable the kernel to learn new configs which
> may be experimental, or otherwise not part of the core set currently
> available through the i915 perf interface.
>
> This will relieve the kernel from holding all the possible configs, so
> we can leave only the test configs here.
>
> Signed-off-by: Matthew Auld 
> Signed-off-by: Lionel Landwerlin 
> Signed-off-by: Andrzej Datczuk 
> ---
>  drivers/gpu/drm/i915/i915_drv.c  |   2 +
>  drivers/gpu/drm/i915/i915_drv.h  |  30 +++
>  drivers/gpu/drm/i915/i915_perf.c | 391 
> ++-
>  drivers/gpu/drm/i915/i915_reg.h  |   2 +
>  include/uapi/drm/i915_drm.h  |  21 +++
>  5 files changed, 441 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index d310d8245dca..a3d339670ec1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2730,6 +2730,8 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
> DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, 
> i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, 
> i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, 
> DRM_RENDER_ALLOW),
> +   DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl, 
> DRM_UNLOCKED|DRM_RENDER_ALLOW),
> +   DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, 
> i915_perf_remove_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
>  };
>
>  static struct drm_driver driver = {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 81cd21ecfa7d..ce733c2db963 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2009,6 +2009,11 @@ struct i915_perf_stream {
>  * type of configured stream.
>  */
> const struct i915_perf_stream_ops *ops;
> +
> +   /**
> +* @metrics_set: The ID of the config used by the stream.
> +*/
> +   int metrics_set;
>  };
>
>  /**
> @@ -2016,6 +2021,25 @@ struct i915_perf_stream {
>   */
>  struct i915_oa_ops {
> /**
> +* @is_valid_b_counter_reg: Validates register's address for
> +* programming boolean counters for a particular platform.
> +*/
> +   bool (*is_valid_b_counter_reg)(struct drm_i915_private *dev_priv,
> +  u32 addr);
> +
> +   /**
> +* @is_valid_mux_reg: Validates register's address for programming mux
> +* for a particular platform.
> +*/
> +   bool (*is_valid_mux_reg)(struct drm_i915_private *dev_priv, u32 addr);
> +
> +   /**
> +* @is_valid_flex_reg: Validates register's address for programming
> +* flex EU filtering for a particular platform.
> +*/
> +   bool (*is_valid_flex_reg)(struct drm_i915_private *dev_priv, u32 
> addr);
> +
> +   /**
>  * @init_oa_buffer: Resets the head and tail pointers of the
>  * circular buffer for periodic OA reports.
>  *
> @@ -2413,6 +2437,8 @@ struct drm_i915_private {
> struct mutex lock;
> struct list_head streams;
>
> +   struct idr metrics_idr;
> +
> struct {
> struct i915_perf_stream *exclusive_stream;
>
> @@ -3589,6 +3615,10 @@ i915_gem_context_lookup_timeline(struct 
> i915_gem_context *ctx,
>
>  int i915_perf_open_ioctl(struct drm_device *dev, void *data,
>  struct drm_file *file);
> +int i915_perf_add_config_ioctl(struct drm_device *dev, void *data,
> +  struct drm_file *file);
> +int i915_perf_remove_config_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *file);
>  void i915_oa_init_reg_state(struct intel_engine_cs *engine,
> struct i915_gem_context *ctx,
> uint32_t *reg_state);
> diff --git a/drivers/gpu/drm/i915/i915_perf.c 
> b/drivers/gpu/drm/i915/i915_perf.c
> index d9f77a4d85db..92cb6a7568e7 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -357,6 +357,23 @@ struct perf_open_properties {
> int oa_period_exponent;
>  };
>
> +struct i915_oa_dynamic_config
> +{
> +   char guid[40];
s/guid/uuid/ to be consistent with elsewhere?

> +   int id;
> +
> +   const struct i915_oa_reg *mux_regs;
> +   int mux_regs_len;
> +   const struct i915_oa_reg *b_counter_regs;
> +   int b_counter_regs_len;
> +  

Re: [Intel-gfx] [RFC] drm/i915/lrc: allocate separate page for HWSP

[Michel Thierry]

On 07/07/17 14:15, Daniele Ceraolo Spurio wrote:
After a bit of investigation I've found that the issue is not actually 
with the positioning of the HWSP but with the fact that we use 
status_page.ggtt_offset to point to the lrca offset of the default 
context in guc_ads_create instead of using 
kernel_context->engine[].state. With that fixed GuC works fine even with 
the HWSP below GUC_WOPCM_TOP.


Agreed, and it's something worth fixing (regardless of what we end up 
deciding about the HWSP). ads.golden_context_lrca shouldn't be relying 
on the HWSP location.


Thanks for spotting this.

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

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [RESEND,v2,1/2] drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated

[Patchwork]

== Series Details ==

Series: series starting with [RESEND,v2,1/2] drm/i915/dp: Generalize 
intel_dp_link_params function to accept arguments to be validated
URL   : https://patchwork.freedesktop.org/series/27028/
State : success

== Summary ==

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

Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
fail   -> PASS   (fi-snb-2600) fdo#17
Test kms_flip:
Subgroup basic-flip-vs-modeset:
skip   -> PASS   (fi-skl-x1585l)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
pass   -> DMESG-WARN (fi-byt-j1900) fdo#101705

fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:438s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:430s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:354s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:538s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:509s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:489s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:488s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:594s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:433s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:412s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:427s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:499s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:475s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:457s
fi-kbl-7560u total:279  pass:268  dwarn:1   dfail:0   fail:0   skip:10  
time:576s
fi-kbl-r total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  
time:581s
fi-pnv-d510  total:279  pass:221  dwarn:3   dfail:0   fail:0   skip:55  
time:566s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:467s
fi-skl-6700hqtotal:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  
time:587s
fi-skl-6700k total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  
time:475s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:485s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:433s
fi-skl-x1585ltotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:510s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:553s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:402s

edc3bde190ad0559ad1c7c63e0efd8d5b4b24b2c drm-tip: 2017y-07m-07d-17h-12m-03s UTC 
integration manifest
edb2eb9 drm/i915/dp: Validate the compliance test link parameters
1d7b5b6 drm/i915/dp: Generalize intel_dp_link_params function to accept 
arguments to be validated

== Logs ==

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

Re: [Intel-gfx] [PATCH 2/4] fbdev: Nuke FBINFO_MODULE

[kbuild test robot]

Hi Daniel,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.12 next-20170707]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Daniel-Vetter/fbcon-Make-fbcon-a-built-time-depency-for-fbdev/20170708-063020
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64 

All error/warnings (new ones prefixed by >>):

   In file included from drivers/video/fbdev/matrox/matroxfb_base.h:35:0,
from drivers/video/fbdev/matrox/matroxfb_base.c:104:
   drivers/video/fbdev/matrox/matroxfb_base.c: In function 'initMatrox2':
>> include/linux/fb.h:538:28: error: 'FBINFO_MODULE' undeclared (first use in 
>> this function)
#define FBINFO_FLAG_MODULE FBINFO_MODULE
   ^
>> drivers/video/fbdev/matrox/matroxfb_base.c:1798:33: note: in expansion of 
>> macro 'FBINFO_FLAG_MODULE'
 minfo->fbcon.flags = hotplug ? FBINFO_FLAG_MODULE : FBINFO_FLAG_DEFAULT;
^~
   include/linux/fb.h:538:28: note: each undeclared identifier is reported only 
once for each function it appears in
#define FBINFO_FLAG_MODULE FBINFO_MODULE
   ^
>> drivers/video/fbdev/matrox/matroxfb_base.c:1798:33: note: in expansion of 
>> macro 'FBINFO_FLAG_MODULE'
 minfo->fbcon.flags = hotplug ? FBINFO_FLAG_MODULE : FBINFO_FLAG_DEFAULT;
^~
--
   In file included from drivers/video//fbdev/matrox/matroxfb_base.h:35:0,
from drivers/video//fbdev/matrox/matroxfb_base.c:104:
   drivers/video//fbdev/matrox/matroxfb_base.c: In function 'initMatrox2':
>> include/linux/fb.h:538:28: error: 'FBINFO_MODULE' undeclared (first use in 
>> this function)
#define FBINFO_FLAG_MODULE FBINFO_MODULE
   ^
   drivers/video//fbdev/matrox/matroxfb_base.c:1798:33: note: in expansion of 
macro 'FBINFO_FLAG_MODULE'
 minfo->fbcon.flags = hotplug ? FBINFO_FLAG_MODULE : FBINFO_FLAG_DEFAULT;
^~
   include/linux/fb.h:538:28: note: each undeclared identifier is reported only 
once for each function it appears in
#define FBINFO_FLAG_MODULE FBINFO_MODULE
   ^
   drivers/video//fbdev/matrox/matroxfb_base.c:1798:33: note: in expansion of 
macro 'FBINFO_FLAG_MODULE'
 minfo->fbcon.flags = hotplug ? FBINFO_FLAG_MODULE : FBINFO_FLAG_DEFAULT;
^~

vim +/FBINFO_MODULE +538 include/linux/fb.h

1471ca9a Marcin Slusarz 2010-05-16  532 a->count = max_num;
1471ca9a Marcin Slusarz 2010-05-16  533 return a;
1471ca9a Marcin Slusarz 2010-05-16  534  }
1471ca9a Marcin Slusarz 2010-05-16  535  
^1da177e Linus Torvalds 2005-04-16  536  
^1da177e Linus Torvalds 2005-04-16  537  // This will go away
^1da177e Linus Torvalds 2005-04-16 @538  #define FBINFO_FLAG_MODULE 
FBINFO_MODULE
^1da177e Linus Torvalds 2005-04-16  539  #define FBINFO_FLAG_DEFAULT
FBINFO_DEFAULT
^1da177e Linus Torvalds 2005-04-16  540  
^1da177e Linus Torvalds 2005-04-16  541  /* This will go away

:: The code at line 538 was first introduced by commit
:: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2

:: TO: Linus Torvalds <torva...@ppc970.osdl.org>
:: CC: Linus Torvalds <torva...@ppc970.osdl.org>

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


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

[Intel-gfx] [RESEND v2 1/2] drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated

[Manasi Navare]

This function now takes the link rate and lane ocunt to be validated
as an argument so that this can be used for validating even the
compliance test link parameters.

Signed-off-by: Manasi Navare 
Cc: Ville Syrjala 
Cc: Jani Nikula 
Reviewed-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_dp.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2d42d09..d94a47c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -322,19 +322,20 @@ static int intel_dp_common_len_rate_limit(struct intel_dp 
*intel_dp,
return 0;
 }
 
-static bool intel_dp_link_params_valid(struct intel_dp *intel_dp)
+static bool intel_dp_link_params_valid(struct intel_dp *intel_dp, int 
link_rate,
+  uint8_t lane_count)
 {
/*
 * FIXME: we need to synchronize the current link parameters with
 * hardware readout. Currently fast link training doesn't work on
 * boot-up.
 */
-   if (intel_dp->link_rate == 0 ||
-   intel_dp->link_rate > intel_dp->max_link_rate)
+   if (link_rate == 0 ||
+   link_rate > intel_dp->max_link_rate)
return false;
 
-   if (intel_dp->lane_count == 0 ||
-   intel_dp->lane_count > intel_dp_max_lane_count(intel_dp))
+   if (lane_count == 0 ||
+   lane_count > intel_dp_max_lane_count(intel_dp))
return false;
 
return true;
@@ -4263,7 +4264,8 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 * Validate the cached values of intel_dp->link_rate and
 * intel_dp->lane_count before attempting to retrain.
 */
-   if (!intel_dp_link_params_valid(intel_dp))
+   if (!intel_dp_link_params_valid(intel_dp, intel_dp->link_rate,
+   intel_dp->lane_count))
return;
 
/* Retrain if Channel EQ or CR not ok */
-- 
2.1.4

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

[Intel-gfx] [RESEND v2 2/2] drm/i915/dp: Validate the compliance test link parameters

[Manasi Navare]

Validate the compliance test link parameters when the compliance
test dpcd registers are read. Also validate them in compute_config
before using them since the max values might have been reduced
due to link training fallback.

If either the link rate or lane count is invalid, we still bail
from using the test parameters since the combination would not work
and instead use the fallback values.

v2:
* Added commit message to explain why we still bail when either of
of the params is invalid (Ville Syrjala)
* Add reason for validating in the comment (Jani Nikula)
* Also check if index >= 0 after validating (Jani Nikula)

Signed-off-by: Manasi Navare 
Cc: Jani Nikula 
Cc: Ville Syrjala 
---
 drivers/gpu/drm/i915/intel_dp.c | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d94a47c..4fc23bc 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1678,12 +1678,18 @@ intel_dp_compute_config(struct intel_encoder *encoder,
if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
int index;
 
-   index = intel_dp_rate_index(intel_dp->common_rates,
-   intel_dp->num_common_rates,
-   
intel_dp->compliance.test_link_rate);
-   if (index >= 0)
-   min_clock = max_clock = index;
-   min_lane_count = max_lane_count = 
intel_dp->compliance.test_lane_count;
+   /* Validate the compliance test data since max values
+* might have changed due to link train fallback.
+*/
+   if (intel_dp_link_params_valid(intel_dp, 
intel_dp->compliance.test_link_rate,
+  
intel_dp->compliance.test_lane_count)) {
+   index = intel_dp_rate_index(intel_dp->common_rates,
+   intel_dp->num_common_rates,
+   
intel_dp->compliance.test_link_rate);
+   if (index >= 0)
+   min_clock = max_clock = index;
+   min_lane_count = max_lane_count = 
intel_dp->compliance.test_lane_count;
+   }
}
DRM_DEBUG_KMS("DP link computation with max lane count %i "
  "max bw %d pixel clock %iKHz\n",
@@ -3964,8 +3970,7 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 
*sink_irq_vector)
 static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
 {
int status = 0;
-   int min_lane_count = 1;
-   int link_rate_index, test_link_rate;
+   int test_link_rate;
uint8_t test_lane_count, test_link_bw;
/* (DP CTS 1.2)
 * 4.3.1.11
@@ -3979,10 +3984,6 @@ static uint8_t intel_dp_autotest_link_training(struct 
intel_dp *intel_dp)
return DP_TEST_NAK;
}
test_lane_count &= DP_MAX_LANE_COUNT_MASK;
-   /* Validate the requested lane count */
-   if (test_lane_count < min_lane_count ||
-   test_lane_count > intel_dp->max_link_lane_count)
-   return DP_TEST_NAK;
 
status = drm_dp_dpcd_readb(_dp->aux, DP_TEST_LINK_RATE,
   _link_bw);
@@ -3990,12 +3991,11 @@ static uint8_t intel_dp_autotest_link_training(struct 
intel_dp *intel_dp)
DRM_DEBUG_KMS("Link Rate read failed\n");
return DP_TEST_NAK;
}
-   /* Validate the requested link rate */
test_link_rate = drm_dp_bw_code_to_link_rate(test_link_bw);
-   link_rate_index = intel_dp_rate_index(intel_dp->common_rates,
- intel_dp->num_common_rates,
- test_link_rate);
-   if (link_rate_index < 0)
+
+   /* Validate the requested link rate and lane count */
+   if (!intel_dp_link_params_valid(intel_dp, test_link_rate,
+   test_lane_count))
return DP_TEST_NAK;
 
intel_dp->compliance.test_lane_count = test_lane_count;
-- 
2.1.4

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

Re: [Intel-gfx] [PATCH IGT v2 4/6] tests/kms_frontbuffer_tracking: Refactor to use IGT PSR library functions

[Jim Bride]

On Fri, Jun 30, 2017 at 01:19:57PM -0700, Rodrigo Vivi wrote:
> I believe it would be better to create the psr lib with only one
> function at time and on every patch that adds the new function already
> removes that from here and from frontbuffer tracking test as well...
> 
> easier to review and to make sure that we are not changing the behaviour.

I'm testing a new series with the requested structural changes and review
feedback to-date.  I hope to send them out on Monday (testing takes a while.)

Jim

> also...
> 
> On Fri, Jun 30, 2017 at 12:12 PM, Jim Bride  wrote:
> > v2: * Minor functional tweaks and bug fixes
> > * Rebase
> >
> > Cc: Rodrigo Vivi 
> > Cc: Paulo Zanoni 
> > Signed-off-by: Jim Bride 
> > ---
> >  tests/kms_frontbuffer_tracking.c | 119 
> > +++
> >  1 file changed, 19 insertions(+), 100 deletions(-)
> >
> > diff --git a/tests/kms_frontbuffer_tracking.c 
> > b/tests/kms_frontbuffer_tracking.c
> > index c24e4a8..3a8b754 100644
> > --- a/tests/kms_frontbuffer_tracking.c
> > +++ b/tests/kms_frontbuffer_tracking.c
> > @@ -183,7 +183,7 @@ struct {
> >  };
> >
> >
> > -#define SINK_CRC_SIZE 12
> > +#define SINK_CRC_SIZE 6
> 
> I believe this deserves a separated patch...
> 
> >  typedef struct {
> > char data[SINK_CRC_SIZE];
> >  } sink_crc_t;
> > @@ -327,28 +327,6 @@ drmModeModeInfo std_1024_mode = {
> > .name = "Custom 1024x768",
> >  };
> >
> > -static drmModeModeInfoPtr get_connector_smallest_mode(drmModeConnectorPtr 
> > c)
> > -{
> > -   int i;
> > -   drmModeModeInfoPtr smallest = NULL;
> > -
> > -   for (i = 0; i < c->count_modes; i++) {
> > -   drmModeModeInfoPtr mode = >modes[i];
> > -
> > -   if (!smallest)
> > -   smallest = mode;
> > -
> > -   if (mode->hdisplay * mode->vdisplay <
> > -   smallest->hdisplay * smallest->vdisplay)
> > -   smallest = mode;
> > -   }
> > -
> > -   if (c->connector_type == DRM_MODE_CONNECTOR_eDP)
> > -   smallest = _1024_mode;
> > -
> > -   return smallest;
> > -}
> > -
> >  static drmModeConnectorPtr get_connector(uint32_t id)
> >  {
> > int i;
> > @@ -421,30 +399,6 @@ static void init_mode_params(struct modeset_params 
> > *params, uint32_t crtc_id,
> > params->sprite.h = 64;
> >  }
> >
> > -static bool connector_get_mode(drmModeConnectorPtr c, drmModeModeInfoPtr 
> > *mode)
> > -{
> > -   *mode = NULL;
> > -
> > -   if (c->connection != DRM_MODE_CONNECTED || !c->count_modes)
> > -   return false;
> > -
> > -   if (c->connector_type == DRM_MODE_CONNECTOR_eDP && opt.no_edp)
> > -   return false;
> > -
> > -   if (opt.small_modes)
> > -   *mode = get_connector_smallest_mode(c);
> > -   else
> > -   *mode = >modes[0];
> > -
> > -/* On HSW the CRC WA is so awful that it makes you think 
> > everything is
> > - * bugged. */
> > -   if (IS_HASWELL(intel_get_drm_devid(drm.fd)) &&
> > -   c->connector_type == DRM_MODE_CONNECTOR_eDP)
> > -   *mode = _1024_mode;
> > -
> > -   return true;
> > -}
> > -
> >  static bool connector_supports_pipe_a(drmModeConnectorPtr connector)
> >  {
> > int i;
> > @@ -473,7 +427,7 @@ static bool find_connector(bool edp_only, bool pipe_a, 
> > uint32_t forbidden_id,
> > continue;
> > if (c->connector_id == forbidden_id)
> > continue;
> > -   if (!connector_get_mode(c, ))
> > +   if (!igt_psr_find_good_mode(c, ))
> > continue;
> >
> > *ret_connector = c;
> > @@ -804,23 +758,6 @@ static void fbc_print_status(void)
> > igt_info("FBC status:\n%s\n", buf);
> >  }
> >
> > -static bool psr_is_enabled(void)
> > -{
> > -   char buf[256];
> > -
> > -   debugfs_read("i915_edp_psr_status", buf);
> > -   return strstr(buf, "\nActive: yes\n") &&
> > -  strstr(buf, "\nHW Enabled & Active bit: yes\n");
> > -}
> > -
> > -static void psr_print_status(void)
> > -{
> > -   char buf[256];
> > -
> > -   debugfs_read("i915_edp_psr_status", buf);
> > -   igt_info("PSR status:\n%s\n", buf);
> > -}
> > -
> >  static struct timespec fbc_get_last_action(void)
> >  {
> > struct timespec ret = { 0, 0 };
> > @@ -926,44 +863,31 @@ static bool fbc_wait_until_enabled(void)
> > return igt_wait(fbc_is_enabled(), 2000, 1);
> >  }
> >
> > -static bool psr_wait_until_enabled(void)
> > -{
> > -   return igt_wait(psr_is_enabled(), 5000, 1);
> > -}
> > -
> >  #define fbc_enable() igt_set_module_param_int("enable_fbc", 1)
> >  #define fbc_disable() igt_set_module_param_int("enable_fbc", 0)
> > -#define psr_enable() igt_set_module_param_int("enable_psr", 1)
> > 

Re: [Intel-gfx] [RFC] drm/i915/lrc: allocate separate page for HWSP

[Daniele Ceraolo Spurio]

On 06/07/17 11:51, Michel Thierry wrote:

On 7/6/2017 10:59 AM, Chris Wilson wrote:
 > If there are no conflicts, then just skip the patch, and really there
 > shouldn't be since the writes we care about are ordered and the writes
 > we don't, we don't. We will need to move to per-context hwsp in the near
 > future which should alleviate these concerns.

It helped me explain the strange data I was seeing in the HSWP during 
driver load came from (that random data wasn't really random, I was 
looking at the PPHWSP which later became the HWSP).


Just a comment/fix for GuC submission below, because as it is, this will 
break it.


-Michel



I'd argue that since we're doing it wrong we should fix it even if it is 
not currently failing, as things could go wrong in unexpected ways 
(especially on new HW). However, I'm happy to drop this patch now if 
we're going to fix it later with the per-context HWSP.






+ */
+flags |= PIN_MAPPABLE;
+ret = i915_vma_pin(vma, 0, 4096, flags);
+if (ret)
+goto err;


This will break GuC submission, the HWSP will be allocated correctly,

[ ] [drm:intel_engine_init_common [i915]] rcs0 hws offset: 0x1000
[ ] [drm:intel_engine_init_common [i915]] bcs0 hws offset: 0x2000
[ ] [drm:intel_engine_init_common [i915]] vcs0 hws offset: 0x3000
[ ] [drm:intel_engine_init_common [i915]] vcs1 hws offset: 0x4000
[ ] [drm:intel_engine_init_common [i915]] vecs0 hws offset: 0x5000

But these are below GUC_WOPCM_TOP (0x8), and our _beloved_ fw must 
be having problems accessing them, because this causes:


[ ] [drm:intel_guc_init_hw [i915]] GuC fw status: fetch SUCCESS, load 
PENDING

[ ] [drm:guc_ucode_xfer_dma [i915]] DMA status 0x10, GuC status 0x800071ec
[ ] [drm:guc_ucode_xfer_dma [i915]] returning -110

So we must need this restriction before pinning it,

@@ -510,6 +510,11 @@ static int init_status_page(struct intel_engine_cs 
*engine)

  * actualy map it).
  */
 flags |= PIN_MAPPABLE;
+
+   /* GuC can only access pages above GUC_WOPCM_TOP ¯\_(ツ)_/¯ */
+   if (HAS_GUC(engine->i915) && i915.enable_guc_loading)
+   flags |= PIN_OFFSET_BIAS | GUC_WOPCM_TOP;
+
 ret = i915_vma_pin(vma, 0, 4096, flags);
 if (ret)
 goto err;

With that change, GuC is happy:

[ ] [drm:intel_engine_init_common [i915]] rcs0 hws offset: 0x00084000
[ ] [drm:intel_engine_init_common [i915]] bcs0 hws offset: 0x00089000
[ ] [drm:intel_engine_init_common [i915]] vcs0 hws offset: 0x0008e000
[ ] [drm:intel_engine_init_common [i915]] vcs1 hws offset: 0x00093000
[ ] [drm:intel_engine_init_common [i915]] vecs0 hws offset: 0x00098000
...
[ ] [drm:intel_guc_init_hw [i915]] GuC fw status: fetch SUCCESS, load 
PENDING

[ ] [drm:guc_ucode_xfer_dma [i915]] DMA status 0x10, GuC status 0x8002f0ec
[ ] [drm:guc_ucode_xfer_dma [i915]] returning 0
[ ] [drm] GuC submission enabled (firmware i915/skl_guc_ver9_33.bin 
[version 9.33])





After a bit of investigation I've found that the issue is not actually 
with the positioning of the HWSP but with the fact that we use 
status_page.ggtt_offset to point to the lrca offset of the default 
context in guc_ads_create instead of using 
kernel_context->engine[].state. With that fixed GuC works fine even with 
the HWSP below GUC_WOPCM_TOP.


Thanks,
Daniele

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

Re: [Intel-gfx] [PATCH IGT v2 5/6] tests/kms_frontbuffer_tracking: Fix multidraw subtest

[Paulo Zanoni]

Em Sex, 2017-06-30 às 12:12 -0700, Jim Bride escreveu:
> The multidraw subtest was not taking whether or not the GEM buffer
> had
> ever been in write-combining mode when checking for PSR state, so fix
> that.

Reviewed-by: Paulo Zanoni 

> 
> Signed-off-by: Jim Bride 
> ---
>  tests/kms_frontbuffer_tracking.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/kms_frontbuffer_tracking.c
> b/tests/kms_frontbuffer_tracking.c
> index 3a8b754..c52d7a0 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -2059,7 +2059,8 @@ static void multidraw_subtest(const struct
> test_mode *t)
>   assertions = used_method !=
> IGT_DRAW_MMAP_GTT ?
>    ASSERT_LAST_ACTION_CHAN
> GED :
>    ASSERT_NO_ACTION_CHANGE
> ;
> - if (op_disables_psr(t, used_method))
> + if (op_disables_psr(t, used_method)
> &&
> + !wc_used)
>   assertions |=
> ASSERT_PSR_DISABLED;
>  
>   do_assertions(assertions);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/1] drm/i915: Version the MOCS settings

[Chris Wilson]

Quoting Ben Widawsky (2017-07-07 19:42:25)
> On 17-07-07 11:34:48, Chris Wilson wrote:
> >Quoting Ben Widawsky (2017-07-07 00:27:01)
> >>  drivers/gpu/drm/i915/i915_drv.c |  3 +++
> >>  drivers/gpu/drm/i915/i915_drv.h |  2 ++
> >>  drivers/gpu/drm/i915/i915_pci.c | 13 +
> >>  include/uapi/drm/i915_drm.h |  8 
> >>  4 files changed, 22 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c 
> >> b/drivers/gpu/drm/i915/i915_drv.c
> >> index 9167a73f3c69..26c27b6ae814 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.c
> >> +++ b/drivers/gpu/drm/i915/i915_drv.c
> >> @@ -401,6 +401,9 @@ static int i915_getparam(struct drm_device *dev, void 
> >> *data,
> >> if (!value)
> >> return -ENODEV;
> >> break;
> >> +   case I915_PARAM_MOCS_TABLE_VERSION:
> >> +   value = INTEL_INFO(dev_priv)->mocs_version;
> >
> >If we use intel_mocs_get_table_version() we can put this magic number
> >in intel_mocs.c next to the tables, where we can keep its history and
> >hopefully be able to remember to update it.
> >
> 
> Yeah, that seems like an improvement to me as well.
> 
> >> +/* What version of the MOCS table we have. For GEN9 GPUs, the PRM defined
> >> + * non-optimal settings for the MOCS table. As a result, we were required 
> >> to use a
> >> + * small subset, and later add new settings. This param allows userspace 
> >> to
> >> + * determine which settings are there.
> >> + */
> >> +#define MOCS_TABLE_VERSION   1 /* Build time MOCS table 
> >> version */
> >
> >How are you planing to share this? When we update we bump this number,
> >and then mesa copies it across and uses it after verifying it as 0,1 on
> >an old kernel.
> >
> >I don't think you want to expose the updated constant here, but symbolic
> >names for each version? (What would be the point?)
> >
> 
> At least one thing wrong here is we would need per GEN constants, which is 
> maybe
> what you meant and I misunderstood. Assuming you had per GEN constants, which 
> I
> don't like, I believe everything works out fine. So, I'll remove this compile
> time MOCS versioning.

I figured you were going towards per-gen versioning, which is kind of
why I liked the idea of table size -- but that only makes sense if
somehow the index has the same meaning across gen (which it won't).

> >Next question, why a version number and not just the number of entries
> >defined? Each index is defined by ABI once assigned, so the number of
> >entries still operates as a version number and allows easy checking.
> >
> >   if (advanced_cacheing_idx < kernel_max_mocs)
> >   return advanced_cacheing_idx;
> >   if (default_cacheing_idx < kernel_max_mocs)
> >   return default_cacheing_idx;
> >
> >   return follow_pte_idx;
> >
> >give or take the smarts to choose the preferred indices for any
> >particular scenario.
> >
> >In the future, if we finally get user defined mocs, the table_size will
> >then give the start of the user modifiable indices (presming they want
> >to keep the predefined entries for compatibility?))
> >-Chris
> 
> Yes, I considered this as well. I see no difference really as to one versus 
> the
> other. In fact, if you're to support multiple table versions, I think it's
> actually easier with a pure version:
> 
> switch (kernel_mocs_version) {
> case 3:
> return new_best_cacheing_index;
> case 2:
> return old_best_cacheing_index;
> case 1:
> return naive_best_index;
> }

Indeed 6 of one, half a dozen of the other. Whichever you pick, 3 years
down the line you wish you picked the other. The big advantage of using
an absolute version is that you can just stuff these into tables. Ok, I
like that more, a version parameter (that may be per-gen) worksforme.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [Mesa-dev] [PATCH 1/1] drm/i915: Version the MOCS settings

[Ben Widawsky]

On 17-07-07 09:23:26, Jason Ekstrand wrote:

On Fri, Jul 7, 2017 at 3:34 AM, Chris Wilson 
wrote:


Quoting Ben Widawsky (2017-07-07 00:27:01)
>  drivers/gpu/drm/i915/i915_drv.c |  3 +++
>  drivers/gpu/drm/i915/i915_drv.h |  2 ++
  drivers/gpu/drm/i915/i915_pci.c | 13 +
>  include/uapi/drm/i915_drm.h |  8 
>  4 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
b/drivers/gpu/drm/i915/i915_drv.c
> index 9167a73f3c69..26c27b6ae814 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -401,6 +401,9 @@ static int i915_getparam(struct drm_device *dev,
void *data,
> if (!value)
> return -ENODEV;
> break;
> +   case I915_PARAM_MOCS_TABLE_VERSION:
> +   value = INTEL_INFO(dev_priv)->mocs_version;

If we use intel_mocs_get_table_version() we can put this magic number
in intel_mocs.c next to the tables, where we can keep its history and
hopefully be able to remember to update it.

> +/* What version of the MOCS table we have. For GEN9 GPUs, the PRM
defined
> + * non-optimal settings for the MOCS table. As a result, we were
required to use a
> + * small subset, and later add new settings. This param allows
userspace to
> + * determine which settings are there.
> + */
> +#define MOCS_TABLE_VERSION   1 /* Build time MOCS table
version */

How are you planing to share this? When we update we bump this number,
and then mesa copies it across and uses it after verifying it as 0,1 on
an old kernel.



Agreed.  I don't see how having a #define for compile-time mocs version is
useful.  The compile-time version doesn't really matter and we wouldn't
want to use that in i965/anv anyway (more on that in the other patch).




I think we're all agreed here.


I don't think you want to expose the updated constant here, but symbolic
names for each version? (What would be the point?)

Next question, why a version number and not just the number of entries
defined? Each index is defined by ABI once assigned, so the number of
entries still operates as a version number and allows easy checking.

if (advanced_cacheing_idx < kernel_max_mocs)
return advanced_cacheing_idx;
if (default_cacheing_idx < kernel_max_mocs)
return default_cacheing_idx;

return follow_pte_idx;

give or take the smarts to choose the preferred indices for any
particular scenario.



I'll have to think about it a bit more but this sounds like a fairly good
idea.  I see two major benefits:

1. The kernel can return ARRAY_SIZE(mocs_table_for_your_gen) and we will
never forget to update it.
2. It makes the "does this MOCS value exist" check much easier.  I imagine
future userspace code which chooses mocs values having some sort of "try
and fall back" approach to making MOCS choices and this would be convenient.

That said, having it be a version may have it's advantages, I just don't
know what they are yet.

--Jason


Please direct comments to my response to Chris if you have more. To me it's 6
one way, half dozen the other - and I believe version has a more direct meaning
(and the ability to potentially, albeit a terrible idea, rewrite entries).

If people are going to block a review based on this, I will change it, but I'd
rather not.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/1] drm/i915: Version the MOCS settings

[Ben Widawsky]

On 17-07-07 11:34:48, Chris Wilson wrote:

Quoting Ben Widawsky (2017-07-07 00:27:01)

 drivers/gpu/drm/i915/i915_drv.c |  3 +++
 drivers/gpu/drm/i915/i915_drv.h |  2 ++
 drivers/gpu/drm/i915/i915_pci.c | 13 +
 include/uapi/drm/i915_drm.h |  8 
 4 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9167a73f3c69..26c27b6ae814 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -401,6 +401,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
if (!value)
return -ENODEV;
break;
+   case I915_PARAM_MOCS_TABLE_VERSION:
+   value = INTEL_INFO(dev_priv)->mocs_version;


If we use intel_mocs_get_table_version() we can put this magic number
in intel_mocs.c next to the tables, where we can keep its history and
hopefully be able to remember to update it.



Yeah, that seems like an improvement to me as well.


+/* What version of the MOCS table we have. For GEN9 GPUs, the PRM defined
+ * non-optimal settings for the MOCS table. As a result, we were required to 
use a
+ * small subset, and later add new settings. This param allows userspace to
+ * determine which settings are there.
+ */
+#define MOCS_TABLE_VERSION   1 /* Build time MOCS table version */


How are you planing to share this? When we update we bump this number,
and then mesa copies it across and uses it after verifying it as 0,1 on
an old kernel.

I don't think you want to expose the updated constant here, but symbolic
names for each version? (What would be the point?)



At least one thing wrong here is we would need per GEN constants, which is maybe
what you meant and I misunderstood. Assuming you had per GEN constants, which I
don't like, I believe everything works out fine. So, I'll remove this compile
time MOCS versioning.


Next question, why a version number and not just the number of entries
defined? Each index is defined by ABI once assigned, so the number of
entries still operates as a version number and allows easy checking.

if (advanced_cacheing_idx < kernel_max_mocs)
return advanced_cacheing_idx;
if (default_cacheing_idx < kernel_max_mocs)
return default_cacheing_idx;

return follow_pte_idx;

give or take the smarts to choose the preferred indices for any
particular scenario.

In the future, if we finally get user defined mocs, the table_size will
then give the start of the user modifiable indices (presming they want
to keep the predefined entries for compatibility?))
-Chris


Yes, I considered this as well. I see no difference really as to one versus the
other. In fact, if you're to support multiple table versions, I think it's
actually easier with a pure version:

switch (kernel_mocs_version) {
case 3:
return new_best_cacheing_index;
case 2:
return old_best_cacheing_index;
case 1:
return naive_best_index;
}
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits

[Pandiyan, Dhinakaran]

iirc I assumed 1 pixel per clk for CNL when I originally submitted the 
workaround patch because cnl_xxx_calc_cdclk() functions assume that. Are we 
missing something in the cdclk setup code to enable 2 pixels per clk?

-DK

From: Ville Syrjälä [ville.syrj...@linux.intel.com]
Sent: Friday, July 07, 2017 11:24 AM
To: Rodrigo Vivi
Cc: intel-gfx; Pandiyan, Dhinakaran; Zanoni, Paulo R; Vivi, Rodrigo
Subject: Re: [Intel-gfx] [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits

On Fri, Jul 07, 2017 at 10:54:47AM -0700, Rodrigo Vivi wrote:
> I will review the series more carefully later,
> but my concern is that with this series applied I got a blank screen on eDP...

Hmm. Oh, I guess we could now be going for a lower cdclk that before
since we now account for 2 pixels per clock factor, whereas previously
didn't. That in itself should be fine of course, but I guess the
difference could be down to the system agent DVFS, which we still don't
handle it seems. So possibly we need to get that sorted before we can
change how CNL picks its cdclk.

>
>
> On Fri, Jul 7, 2017 at 3:24 AM,   wrote:
> > From: Ville Syrjälä 
> >
> > Follow the GLK path when computing cdclk and related limits. CNL
> > pipes also produce two pixels per clock, so that's what we should
> > use.
> >
> > For the HBR2 vs. audio issue the limit should more correctly be 336
> > MHz, but the GLK limit of 316.8 MHz works just as well and results
> > in picking at least 336 MHz. Also toss in some related w/a numbers.
> >
> > Cc: Paulo Zanoni 
> > Cc: Rodrigo Vivi 
> > Cc: Dhinakaran Pandiyan 
> > Cc: Maarten Lankhorst 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/intel_cdclk.c | 15 +--
> >  1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c 
> > b/drivers/gpu/drm/i915/intel_cdclk.c
> > index 1241e5891b29..9e0deebae279 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct 
> > intel_crtc_state *crtc_state,
> > crtc_state->has_audio &&
> > crtc_state->port_clock >= 54 &&
> > crtc_state->lane_count == 4) {
> > -   if (IS_CANNONLAKE(dev_priv))
> > -   pixel_rate = max(316800, pixel_rate);
> > -   else if (IS_GEMINILAKE(dev_priv))
> > +   if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> > +   /* Display WA #1145: glk,cnl */
> > pixel_rate = max(2 * 316800, pixel_rate);
> > -   else
> > +   } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> > +   /* Display WA #1144: skl,bxt */
> > pixel_rate = max(432000, pixel_rate);
> > +   }
> > }
> >
> > /* According to BSpec, "The CD clock frequency must be at least 
> > twice
> > @@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct 
> > intel_crtc_state *crtc_state,
> >  * two pixels per clock.
> >  */
> > if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
> > -   if (IS_GEMINILAKE(dev_priv))
> > +   if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > pixel_rate = max(2 * 2 * 96000, pixel_rate);
> > else
> > pixel_rate = max(2 * 96000, pixel_rate);
> > @@ -1999,7 +2000,9 @@ static int intel_compute_max_dotclk(struct 
> > drm_i915_private *dev_priv)
> >  {
> > int max_cdclk_freq = dev_priv->max_cdclk_freq;
> >
> > -   if (IS_GEMINILAKE(dev_priv))
> > +   if (IS_CANNONLAKE(dev_priv))
> > +   return 2 * max_cdclk_freq;
> > +   else if (IS_GEMINILAKE(dev_priv))
> > /*
> >  * FIXME: Limiting to 99% as a temporary workaround. See
> >  * glk_calc_cdclk() for details.
> > --
> > 2.13.0
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br

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

Re: [Intel-gfx] [PATCH 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock"

[Ville Syrjälä]

On Fri, Jul 07, 2017 at 01:24:49PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Make the min_pixclk thing less confusing by changing it to track
> the minimum acceptable cdclk frequency instead. This means moving
> the application of the guardbands to a slightly higher level from
> the low level platform specific calc_cdclk() functions.
> 
> The immediate benefit is elimination of the confusing 2x factors
> on GLK/CNL+ in the audio workarounds (which stems from the fact
> that the pipes produce two pixels per clock).
> 
> Cc: Paulo Zanoni 
> Cc: Rodrigo Vivi 
> Cc: Dhinakaran Pandiyan 
> Cc: Maarten Lankhorst 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  12 ++-
>  drivers/gpu/drm/i915/intel_cdclk.c   | 179 
> +--
>  drivers/gpu/drm/i915/intel_display.c |  21 ++--
>  drivers/gpu/drm/i915/intel_drv.h |   4 +-
>  4 files changed, 107 insertions(+), 109 deletions(-)
>

> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c 
> b/drivers/gpu/drm/i915/intel_cdclk.c
> index 9e0deebae279..82e5bc967cca 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1732,21 +1723,47 @@ void intel_set_cdclk(struct drm_i915_private 
> *dev_priv,
>   dev_priv->display.set_cdclk(dev_priv, cdclk_state);
>  }
>  
> -static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state 
> *crtc_state,
> -   int pixel_rate)
> +static int intel_min_cdclk(struct drm_i915_private *dev_priv,
> +int pixel_rate)
> +{
> + if (INTEL_GEN(dev_priv) >= 10)
> + return DIV_ROUND_UP(pixel_rate, 2);

Rodrigo, so this part here could be why your CNL no longer works.

If you have time to try it, then I think changing this to just
'return pixel_rate;' should get us back to the old behaviour.

> + else if (IS_GEMINILAKE(dev_priv))
> + /*
> +  * FIXME: Avoid using a pixel clock that is more than 99% of 
> the cdclk
> +  * as a temporary workaround. Use a higher cdclk instead. (Note 
> that
> +  * intel_compute_max_dotclk() limits the max pixel clock to 99% 
> of max
> +  * cdclk.)
> +  */
> + return DIV_ROUND_UP(pixel_rate * 100, 2 * 99);
> + else if (IS_GEN9(dev_priv) ||
> +  IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
> + return pixel_rate;
> + else if (IS_CHERRYVIEW(dev_priv))
> + return DIV_ROUND_UP(pixel_rate * 100, 95);
> + else
> + return DIV_ROUND_UP(pixel_rate * 100, 90);
> +}
> +

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

Re: [Intel-gfx] [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits

[Ville Syrjälä]

On Fri, Jul 07, 2017 at 10:54:47AM -0700, Rodrigo Vivi wrote:
> I will review the series more carefully later,
> but my concern is that with this series applied I got a blank screen on eDP...

Hmm. Oh, I guess we could now be going for a lower cdclk that before
since we now account for 2 pixels per clock factor, whereas previously
didn't. That in itself should be fine of course, but I guess the
difference could be down to the system agent DVFS, which we still don't
handle it seems. So possibly we need to get that sorted before we can
change how CNL picks its cdclk.

> 
> 
> On Fri, Jul 7, 2017 at 3:24 AM,   wrote:
> > From: Ville Syrjälä 
> >
> > Follow the GLK path when computing cdclk and related limits. CNL
> > pipes also produce two pixels per clock, so that's what we should
> > use.
> >
> > For the HBR2 vs. audio issue the limit should more correctly be 336
> > MHz, but the GLK limit of 316.8 MHz works just as well and results
> > in picking at least 336 MHz. Also toss in some related w/a numbers.
> >
> > Cc: Paulo Zanoni 
> > Cc: Rodrigo Vivi 
> > Cc: Dhinakaran Pandiyan 
> > Cc: Maarten Lankhorst 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/intel_cdclk.c | 15 +--
> >  1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c 
> > b/drivers/gpu/drm/i915/intel_cdclk.c
> > index 1241e5891b29..9e0deebae279 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct 
> > intel_crtc_state *crtc_state,
> > crtc_state->has_audio &&
> > crtc_state->port_clock >= 54 &&
> > crtc_state->lane_count == 4) {
> > -   if (IS_CANNONLAKE(dev_priv))
> > -   pixel_rate = max(316800, pixel_rate);
> > -   else if (IS_GEMINILAKE(dev_priv))
> > +   if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> > +   /* Display WA #1145: glk,cnl */
> > pixel_rate = max(2 * 316800, pixel_rate);
> > -   else
> > +   } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> > +   /* Display WA #1144: skl,bxt */
> > pixel_rate = max(432000, pixel_rate);
> > +   }
> > }
> >
> > /* According to BSpec, "The CD clock frequency must be at least 
> > twice
> > @@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct 
> > intel_crtc_state *crtc_state,
> >  * two pixels per clock.
> >  */
> > if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
> > -   if (IS_GEMINILAKE(dev_priv))
> > +   if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > pixel_rate = max(2 * 2 * 96000, pixel_rate);
> > else
> > pixel_rate = max(2 * 96000, pixel_rate);
> > @@ -1999,7 +2000,9 @@ static int intel_compute_max_dotclk(struct 
> > drm_i915_private *dev_priv)
> >  {
> > int max_cdclk_freq = dev_priv->max_cdclk_freq;
> >
> > -   if (IS_GEMINILAKE(dev_priv))
> > +   if (IS_CANNONLAKE(dev_priv))
> > +   return 2 * max_cdclk_freq;
> > +   else if (IS_GEMINILAKE(dev_priv))
> > /*
> >  * FIXME: Limiting to 99% as a temporary workaround. See
> >  * glk_calc_cdclk() for details.
> > --
> > 2.13.0
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br

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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Force CPU synchronisation even if userspace requests ASYNC

[Patchwork]

== Series Details ==

Series: drm/i915: Force CPU synchronisation even if userspace requests ASYNC
URL   : https://patchwork.freedesktop.org/series/27012/
State : success

== Summary ==

Series 27012v1 drm/i915: Force CPU synchronisation even if userspace requests 
ASYNC
https://patchwork.freedesktop.org/api/1.0/series/27012/revisions/1/mbox/

Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
fail   -> PASS   (fi-snb-2600) fdo#17
Test gem_exec_suspend:
Subgroup basic-s4-devices:
dmesg-warn -> PASS   (fi-kbl-7560u) fdo#100125 +1
Test kms_flip:
Subgroup basic-flip-vs-modeset:
skip   -> PASS   (fi-skl-x1585l)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
dmesg-warn -> PASS   (fi-pnv-d510) fdo#101597
Subgroup suspend-read-crc-pipe-b:
pass   -> DMESG-WARN (fi-byt-j1900) fdo#101705

fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:438s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:431s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:355s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:527s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:508s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:494s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:483s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:601s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:436s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:416s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:423s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:502s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:477s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:461s
fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:567s
fi-kbl-r total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:581s
fi-pnv-d510  total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  
time:552s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:458s
fi-skl-6700hqtotal:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  
time:584s
fi-skl-6700k total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  
time:470s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:482s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:436s
fi-skl-x1585ltotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:496s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:542s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:407s

edc3bde190ad0559ad1c7c63e0efd8d5b4b24b2c drm-tip: 2017y-07m-07d-17h-12m-03s UTC 
integration manifest
5c70316 drm/i915: Force CPU synchronisation even if userspace requests ASYNC

== Logs ==

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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/perf: Add support for loadable OA configs

[Patchwork]

== Series Details ==

Series: drm/i915/perf: Add support for loadable OA configs
URL   : https://patchwork.freedesktop.org/series/27011/
State : success

== Summary ==

Series 27011v1 drm/i915/perf: Add support for loadable OA configs
https://patchwork.freedesktop.org/api/1.0/series/27011/revisions/1/mbox/

Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
fail   -> PASS   (fi-snb-2600) fdo#17
Test kms_flip:
Subgroup basic-flip-vs-modeset:
skip   -> PASS   (fi-skl-x1585l)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
pass   -> DMESG-WARN (fi-byt-j1900) fdo#101705

fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:446s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:429s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:354s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:526s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:512s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:491s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:484s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:591s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:432s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:411s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:415s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:496s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:478s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:465s
fi-kbl-7560u total:279  pass:268  dwarn:1   dfail:0   fail:0   skip:10  
time:575s
fi-kbl-r total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  
time:581s
fi-pnv-d510  total:279  pass:221  dwarn:3   dfail:0   fail:0   skip:55  
time:559s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:456s
fi-skl-6700hqtotal:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  
time:585s
fi-skl-6700k total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  
time:466s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:484s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:438s
fi-skl-x1585ltotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:494s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:551s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:407s

edc3bde190ad0559ad1c7c63e0efd8d5b4b24b2c drm-tip: 2017y-07m-07d-17h-12m-03s UTC 
integration manifest
4ac99e7 drm/i915/perf: prune OA configs
c1f5343 drm/i915: Implement I915_PERF_ADD/REMOVE_CONFIG interface

== Logs ==

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

Re: [Intel-gfx] [PATCH] drm/i915/cnl: Add max allowed Cannonlake DC.

[Imre Deak]

On Thu, Jul 06, 2017 at 01:45:08PM -0700, Rodrigo Vivi wrote:
> This is a follow-up after enabling DC states with
> commit: "drm/i915/DMC/CNL: Load DMC on CNL".
> 
> Cc: Anusha Srivatsa 
> Cc: Imre Deak 
> Signed-off-by: Rodrigo Vivi 

Reviewed-by: Imre Deak 

We could add max_dc_state to intel_device_info at one point.

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 5eb9c5e..f630d63 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -2492,7 +2492,7 @@ static uint32_t get_allowed_dc_mask(const struct 
> drm_i915_private *dev_priv,
>   int requested_dc;
>   int max_dc;
>  
> - if (IS_GEN9_BC(dev_priv)) {
> + if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv)) {
>   max_dc = 2;
>   mask = 0;
>   } else if (IS_GEN9_LP(dev_priv)) {
> -- 
> 1.9.1
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits

[Rodrigo Vivi]

I will review the series more carefully later,
but my concern is that with this series applied I got a blank screen on eDP...


On Fri, Jul 7, 2017 at 3:24 AM,   wrote:
> From: Ville Syrjälä 
>
> Follow the GLK path when computing cdclk and related limits. CNL
> pipes also produce two pixels per clock, so that's what we should
> use.
>
> For the HBR2 vs. audio issue the limit should more correctly be 336
> MHz, but the GLK limit of 316.8 MHz works just as well and results
> in picking at least 336 MHz. Also toss in some related w/a numbers.
>
> Cc: Paulo Zanoni 
> Cc: Rodrigo Vivi 
> Cc: Dhinakaran Pandiyan 
> Cc: Maarten Lankhorst 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_cdclk.c | 15 +--
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c 
> b/drivers/gpu/drm/i915/intel_cdclk.c
> index 1241e5891b29..9e0deebae279 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct 
> intel_crtc_state *crtc_state,
> crtc_state->has_audio &&
> crtc_state->port_clock >= 54 &&
> crtc_state->lane_count == 4) {
> -   if (IS_CANNONLAKE(dev_priv))
> -   pixel_rate = max(316800, pixel_rate);
> -   else if (IS_GEMINILAKE(dev_priv))
> +   if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> +   /* Display WA #1145: glk,cnl */
> pixel_rate = max(2 * 316800, pixel_rate);
> -   else
> +   } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> +   /* Display WA #1144: skl,bxt */
> pixel_rate = max(432000, pixel_rate);
> +   }
> }
>
> /* According to BSpec, "The CD clock frequency must be at least twice
> @@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct 
> intel_crtc_state *crtc_state,
>  * two pixels per clock.
>  */
> if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
> -   if (IS_GEMINILAKE(dev_priv))
> +   if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> pixel_rate = max(2 * 2 * 96000, pixel_rate);
> else
> pixel_rate = max(2 * 96000, pixel_rate);
> @@ -1999,7 +2000,9 @@ static int intel_compute_max_dotclk(struct 
> drm_i915_private *dev_priv)
>  {
> int max_cdclk_freq = dev_priv->max_cdclk_freq;
>
> -   if (IS_GEMINILAKE(dev_priv))
> +   if (IS_CANNONLAKE(dev_priv))
> +   return 2 * max_cdclk_freq;
> +   else if (IS_GEMINILAKE(dev_priv))
> /*
>  * FIXME: Limiting to 99% as a temporary workaround. See
>  * glk_calc_cdclk() for details.
> --
> 2.13.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Implement I915_PERF_ADD/REMOVE_CONFIG interface

[Chris Wilson]

Quoting Lionel Landwerlin (2017-07-07 18:08:37)
> +static bool gen8_is_valid_flex_addr(struct drm_i915_private *dev_priv, u32 
> addr)
> +{
> +   static const i915_reg_t flex_eu_regs[] = {
> +   EU_PERF_CNTL0,
> +   EU_PERF_CNTL1,
> +   EU_PERF_CNTL2,
> +   EU_PERF_CNTL3,
> +   EU_PERF_CNTL4,
> +   EU_PERF_CNTL5,
> +   EU_PERF_CNTL6,
> +   };
> +   int i;
> +
> +   for (i = 0; i < ARRAY_SIZE(flex_eu_regs); i++) {
> +   if (flex_eu_regs[i].reg == addr)
> +   return true;
> +   }
> +   return false;
> +}
> +
> +static bool gen7_is_valid_b_counter_addr(struct drm_i915_private *dev_priv, 
> u32 addr)
> +{
> +   return (addr >= 0x2380 && addr <= 0x27ac);
> +}
> +
> +static bool gen7_is_valid_mux_addr(struct drm_i915_private *dev_priv, u32 
> addr)
> +{
> +   return addr == NOA_WRITE.reg ||
> +   (addr >= 0xd0c && addr <= 0xd3c) ||
> +   (addr >= 0x25100 && addr <= 0x2FB9C);
> +}
> +
> +static bool hsw_is_valid_mux_addr(struct drm_i915_private *dev_priv, u32 
> addr)
> +{
> +   return (addr >= 0x25100 && addr <= 0x2FF90) ||
> +   gen7_is_valid_mux_addr(dev_priv, addr);
> +}
> +
> +static bool chv_is_valid_mux_addr(struct drm_i915_private *dev_priv, u32 
> addr)
> +{
> +   return (addr >= 0x182300 && addr <= 0x1823A4) ||
> +   gen7_is_valid_mux_addr(dev_priv, addr);
> +}

Looks like you've already thought of what I was about to say: we need
whitelisting of what userspace can read/tweak. It's a nuisance, but we
could let the privileged (oa_paranoid?) client load an arbitrary config
(though again that may have to be within reason).
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] Skylake / (EE) modeset(0): present flip failed loop

[Marc MERLIN]

On Fri, Jul 07, 2017 at 11:47:25AM +0100, Chris Wilson wrote:
> Quoting Marc MERLIN (2017-07-07 06:40:51)
> > Is this the right place to send this?
> > Can anyone help?
> > 
> > On Wed, Jul 05, 2017 at 11:33:01PM -0700, Marc MERLIN wrote:
> > > Howdy,
> > > 
> > > I have a thinkpad P70 with debian testing and 4.11.6 kernel.
> > > A recent-ish upgrade broke something and now I'm getting loads of spam
> > > in my Xorg.log
> > > 
> > > [  5031.435] (WW) modeset(0): flip queue failed: Invalid argument
> > > [  5031.435] (WW) modeset(0): Page flip failed: Invalid argument
> > > [  5031.435] (EE) modeset(0): present flip failed
> > > [  5031.519] (WW) modeset(0): flip queue failed: Invalid argument
> > > [  5031.519] (WW) modeset(0): Page flip failed: Invalid argument
> > > [  5031.519] (EE) modeset(0): present flip failed
> > > (...)
> > > 
> > > system info:
> > > ii  libdrm-intel1:amd642.4.74-1   
> > > ii  xserver-xorg-core  2:1.19.2-1  
> > > ii  xserver-xorg-video-intel   2:2.99.917+git20161206-1
> 
> If you were indeed using -intel then I would be more concerned.
 
Thanks for the reply.
Sorry, I'm not quite parsing what you wrote here. Are you saying that I
should be disable the modesetting driver?
To be honest, I didn't actually choose it, it seems that Debian forced
the switch to it.

xserver-xorg-video-intel (2:2.13.0-2) unstable; urgency=low

  * Starting from 2.10, the Intel X driver depends on a kernel driver for
mode setting (that's called KMS). The corresponding kernel option is
CONFIG_DRM_I915, and is enabled in Debian kernels.
  * To enable KMS, either of those should be sufficient:
 + /etc/modprobe.d/i915-kms.conf should contain:
 options i915 modeset=1

If so, how do you recommend I switch back if that's what you meant I should do?

> But at the very least you need to dig into dmesg (with drm.debug=fe) to
> find out why it failed. (One way is to run -intel with debugging enabled
> so that it includes the kernel error messages along with the failure
> message.)

Sounds like I need to switch drivers?
Right now I have no xorg.conf and it just autodetects/sets the KMS driver.
Sorry if I'm kind of a NOOB here, but if you give me a short pointer to 
how you'd like me to switch, I'll happily do so.

Thanks,
Marc
-- 
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems 
   what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/ | PGP 1024R/763BE901
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Force CPU synchronisation even if userspace requests ASYNC

[Chris Wilson]

The goal here was to minimise doing any thing or any check inside the
kernel that was not strictly required. For a userspace that assumes
complete control over the cache domains, the kernel is usually using
outdated information and may trigger clflushes where none were
required.

However, swapping is a situation where userspace has no knowledge of the
domain transfer, and will leave the object in the CPU cache. The kernel
must flush this out to the backing storage prior to use with the GPU. As
we use an asynchronous task tracked by an implicit fence for this, we
also need to cancel the ASYNC flag on the object so that the object will
wait for the clflush to complete before being executed. This also absolves
userspace of the responsibility imposed by commit 77ae9957897d ("drm/i915:
Enable userspace to opt-out of implicit fencing") that its needed to ensure
that the object was out of the CPU cache prior to use on the GPU.

Fixes: 77ae9957897d ("drm/i915: Enable userspace to opt-out of implicit 
fencing")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101571
Signed-off-by: Chris Wilson 
Cc: Joonas Lahtinen 
Cc: Jason Ekstrand 
---
 drivers/gpu/drm/i915/i915_gem_clflush.c|  7 ---
 drivers/gpu/drm/i915/i915_gem_clflush.h|  2 +-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 10 ++
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c 
b/drivers/gpu/drm/i915/i915_gem_clflush.c
index 152f16c11878..348b29a845c9 100644
--- a/drivers/gpu/drm/i915/i915_gem_clflush.c
+++ b/drivers/gpu/drm/i915/i915_gem_clflush.c
@@ -114,7 +114,7 @@ i915_clflush_notify(struct i915_sw_fence *fence,
return NOTIFY_DONE;
 }
 
-void i915_gem_clflush_object(struct drm_i915_gem_object *obj,
+bool i915_gem_clflush_object(struct drm_i915_gem_object *obj,
 unsigned int flags)
 {
struct clflush *clflush;
@@ -128,7 +128,7 @@ void i915_gem_clflush_object(struct drm_i915_gem_object 
*obj,
 */
if (!i915_gem_object_has_struct_page(obj)) {
obj->cache_dirty = false;
-   return;
+   return false;
}
 
/* If the GPU is snooping the contents of the CPU cache,
@@ -140,7 +140,7 @@ void i915_gem_clflush_object(struct drm_i915_gem_object 
*obj,
 * tracking.
 */
if (!(flags & I915_CLFLUSH_FORCE) && obj->cache_coherent)
-   return;
+   return false;
 
trace_i915_gem_object_clflush(obj);
 
@@ -179,4 +179,5 @@ void i915_gem_clflush_object(struct drm_i915_gem_object 
*obj,
}
 
obj->cache_dirty = false;
+   return true;
 }
diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.h 
b/drivers/gpu/drm/i915/i915_gem_clflush.h
index 2455a7820937..f390247561b3 100644
--- a/drivers/gpu/drm/i915/i915_gem_clflush.h
+++ b/drivers/gpu/drm/i915/i915_gem_clflush.h
@@ -28,7 +28,7 @@
 struct drm_i915_private;
 struct drm_i915_gem_object;
 
-void i915_gem_clflush_object(struct drm_i915_gem_object *obj,
+bool i915_gem_clflush_object(struct drm_i915_gem_object *obj,
 unsigned int flags);
 #define I915_CLFLUSH_FORCE BIT(0)
 #define I915_CLFLUSH_SYNC BIT(1)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 2c76d6980855..aeacfe9a0878 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1826,7 +1826,7 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
int err;
 
for (i = 0; i < count; i++) {
-   const struct drm_i915_gem_exec_object2 *entry = >exec[i];
+   struct drm_i915_gem_exec_object2 *entry = >exec[i];
struct i915_vma *vma = exec_to_vma(entry);
struct drm_i915_gem_object *obj = vma->obj;
 
@@ -1842,12 +1842,14 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
eb->request->capture_list = capture;
}
 
+   if (unlikely(obj->cache_dirty && !obj->cache_coherent)) {
+   if (i915_gem_clflush_object(obj, 0))
+   entry->flags &= ~EXEC_OBJECT_ASYNC;
+   }
+
if (entry->flags & EXEC_OBJECT_ASYNC)
goto skip_flushes;
 
-   if (unlikely(obj->cache_dirty && !obj->cache_coherent))
-   i915_gem_clflush_object(obj, 0);
-
err = i915_gem_request_await_object
(eb->request, obj, entry->flags & EXEC_OBJECT_WRITE);
if (err)
-- 
2.13.2

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

[Intel-gfx] [PATCH 1/2] drm/i915: Implement I915_PERF_ADD/REMOVE_CONFIG interface

[Lionel Landwerlin]

From: Matthew Auld 

The motivation behind this new interface is expose at runtime the
creation of new OA configs which can be used as part of the i915 perf
open interface. This will enable the kernel to learn new configs which
may be experimental, or otherwise not part of the core set currently
available through the i915 perf interface.

This will relieve the kernel from holding all the possible configs, so
we can leave only the test configs here.

Signed-off-by: Matthew Auld 
Signed-off-by: Lionel Landwerlin 
Signed-off-by: Andrzej Datczuk 
---
 drivers/gpu/drm/i915/i915_drv.c  |   2 +
 drivers/gpu/drm/i915/i915_drv.h  |  30 +++
 drivers/gpu/drm/i915/i915_perf.c | 391 ++-
 drivers/gpu/drm/i915/i915_reg.h  |   2 +
 include/uapi/drm/i915_drm.h  |  21 +++
 5 files changed, 441 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d310d8245dca..a3d339670ec1 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2730,6 +2730,8 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, 
i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, 
i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, 
DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl, 
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, 
i915_perf_remove_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 };
 
 static struct drm_driver driver = {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 81cd21ecfa7d..ce733c2db963 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2009,6 +2009,11 @@ struct i915_perf_stream {
 * type of configured stream.
 */
const struct i915_perf_stream_ops *ops;
+
+   /**
+* @metrics_set: The ID of the config used by the stream.
+*/
+   int metrics_set;
 };
 
 /**
@@ -2016,6 +2021,25 @@ struct i915_perf_stream {
  */
 struct i915_oa_ops {
/**
+* @is_valid_b_counter_reg: Validates register's address for
+* programming boolean counters for a particular platform.
+*/
+   bool (*is_valid_b_counter_reg)(struct drm_i915_private *dev_priv,
+  u32 addr);
+
+   /**
+* @is_valid_mux_reg: Validates register's address for programming mux
+* for a particular platform.
+*/
+   bool (*is_valid_mux_reg)(struct drm_i915_private *dev_priv, u32 addr);
+
+   /**
+* @is_valid_flex_reg: Validates register's address for programming
+* flex EU filtering for a particular platform.
+*/
+   bool (*is_valid_flex_reg)(struct drm_i915_private *dev_priv, u32 addr);
+
+   /**
 * @init_oa_buffer: Resets the head and tail pointers of the
 * circular buffer for periodic OA reports.
 *
@@ -2413,6 +2437,8 @@ struct drm_i915_private {
struct mutex lock;
struct list_head streams;
 
+   struct idr metrics_idr;
+
struct {
struct i915_perf_stream *exclusive_stream;
 
@@ -3589,6 +3615,10 @@ i915_gem_context_lookup_timeline(struct i915_gem_context 
*ctx,
 
 int i915_perf_open_ioctl(struct drm_device *dev, void *data,
 struct drm_file *file);
+int i915_perf_add_config_ioctl(struct drm_device *dev, void *data,
+  struct drm_file *file);
+int i915_perf_remove_config_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file);
 void i915_oa_init_reg_state(struct intel_engine_cs *engine,
struct i915_gem_context *ctx,
uint32_t *reg_state);
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index d9f77a4d85db..92cb6a7568e7 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -357,6 +357,23 @@ struct perf_open_properties {
int oa_period_exponent;
 };
 
+struct i915_oa_dynamic_config
+{
+   char guid[40];
+   int id;
+
+   const struct i915_oa_reg *mux_regs;
+   int mux_regs_len;
+   const struct i915_oa_reg *b_counter_regs;
+   int b_counter_regs_len;
+   const struct i915_oa_reg *flex_regs;
+   int flex_regs_len;
+
+   struct attribute_group sysfs_metric;
+   struct attribute *attrs[2];
+   struct device_attribute sysfs_metric_id;
+};
+
 static u32 gen8_oa_hw_tail_read(struct drm_i915_private *dev_priv)
 {
return I915_READ(GEN8_OATAILPTR) & GEN8_OATAILPTR_MASK;
@@ -1451,11 +1468,39 

[Intel-gfx] [PATCH 0/2] drm/i915/perf: Add support for loadable OA configs

[Lionel Landwerlin]

Hi,

This series adds support for loadable OA configurations. It is based
off Matthew's initial work. This idea is to let userspace mostly hold
these configurations, so they can be tweaked from userspace without
patching the kernel, which is quite useful for debug purposes.

I've made sent IGT tests for it :

   https://patchwork.freedesktop.org/series/27010/

Though this series depends on a bunch of commits not fully reviewed
yet. You can find the branch here :

   https://github.com/djdeath/intel-gpu-tools/tree/wip/djdeath/oa-next

I've also tested this with GPUTop :

https://github.com/rib/gputop/commits/for-master

Eventually I'll produce the patches for Mesa and we'll be able to
remove most of the config from kernel space. I think it's best to
leave the test configs which are quite useful to verify stuff works as
expected, but if people feel they should go too, I'm okay with that.

Cheers,

Lionel Landwerlin (1):
  drm/i915/perf: prune OA configs

Matthew Auld (1):
  drm/i915: Implement I915_PERF_ADD/REMOVE_CONFIG interface

 drivers/gpu/drm/i915/i915_drv.c   |2 +
 drivers/gpu/drm/i915/i915_drv.h   |   30 +
 drivers/gpu/drm/i915/i915_oa_bdw.c| 5351 +
 drivers/gpu/drm/i915/i915_oa_bxt.c| 2566 +---
 drivers/gpu/drm/i915/i915_oa_chv.c| 2763 +
 drivers/gpu/drm/i915/i915_oa_glk.c| 2478 +--
 drivers/gpu/drm/i915/i915_oa_hsw.c|  649 +---
 drivers/gpu/drm/i915/i915_oa_kblgt2.c | 2876 +-
 drivers/gpu/drm/i915/i915_oa_kblgt3.c | 2925 +-
 drivers/gpu/drm/i915/i915_oa_sklgt2.c | 3363 +
 drivers/gpu/drm/i915/i915_oa_sklgt3.c | 2924 +-
 drivers/gpu/drm/i915/i915_oa_sklgt4.c | 2978 +-
 drivers/gpu/drm/i915/i915_perf.c  |  391 ++-
 drivers/gpu/drm/i915/i915_reg.h   |2 +
 include/uapi/drm/i915_drm.h   |   21 +
 15 files changed, 811 insertions(+), 28508 deletions(-)

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

Re: [Intel-gfx] [PATCH] drm/i915/cnl: Get DDI clock based on PLLs.

[Rodrigo Vivi]

patch merged to dinq.

On Thu, Jul 6, 2017 at 1:52 PM, Rodrigo Vivi  wrote:
> PLLs are the source clocks for the DDIs so in order
> to determine the ddi clock we need to check the PLL
> configuration.
>
> v2: Mika pointed out that 24 was hardcoded while it
> should consider ref clock that can be either 24KHz
> or 19.2KHz on CNL.
>
> Reviewed-by: Mika Kahola 
> Signed-off-by: Rodrigo Vivi 
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |   2 +
>  drivers/gpu/drm/i915/intel_ddi.c | 111 
> +++
>  2 files changed, 113 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 64cc674..d6b537e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8343,6 +8343,7 @@ enum {
>  #define  DPLL_CFGCR0_LINK_RATE_3240(6 << 25)
>  #define  DPLL_CFGCR0_LINK_RATE_4050(7 << 25)
>  #define  DPLL_CFGCR0_DCO_FRACTION_MASK (0x7fff << 10)
> +#define  DPLL_CFGCR0_DCO_FRAC_SHIFT(10)
>  #define  DPLL_CFGCR0_DCO_FRACTION(x)   ((x) << 10)
>  #define  DPLL_CFGCR0_DCO_INTEGER_MASK  (0x3ff)
>  #define CNL_DPLL_CFGCR0(pll)   _MMIO_PLL(pll, _CNL_DPLL0_CFGCR0, 
> _CNL_DPLL1_CFGCR0)
> @@ -8350,6 +8351,7 @@ enum {
>  #define _CNL_DPLL0_CFGCR1  0x6C004
>  #define _CNL_DPLL1_CFGCR1  0x6C084
>  #define  DPLL_CFGCR1_QDIV_RATIO_MASK   (0xff << 10)
> +#define  DPLL_CFGCR1_QDIV_RATIO_SHIFT  (10)
>  #define  DPLL_CFGCR1_QDIV_RATIO(x) ((x) << 10)
>  #define  DPLL_CFGCR1_QDIV_MODE(x)  ((x) << 9)
>  #define  DPLL_CFGCR1_KDIV_MASK (7 << 6)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 80e96f1..241decf 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1103,6 +1103,62 @@ static int skl_calc_wrpll_link(struct drm_i915_private 
> *dev_priv,
> return dco_freq / (p0 * p1 * p2 * 5);
>  }
>
> +static int cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
> +  uint32_t pll_id)
> +{
> +   uint32_t cfgcr0, cfgcr1;
> +   uint32_t p0, p1, p2, dco_freq, ref_clock;
> +
> +   cfgcr0 = I915_READ(CNL_DPLL_CFGCR0(pll_id));
> +   cfgcr1 = I915_READ(CNL_DPLL_CFGCR1(pll_id));
> +
> +   p0 = cfgcr1 & DPLL_CFGCR1_PDIV_MASK;
> +   p2 = cfgcr1 & DPLL_CFGCR1_KDIV_MASK;
> +
> +   if (cfgcr1 & DPLL_CFGCR1_QDIV_MODE(1))
> +   p1 = (cfgcr1 & DPLL_CFGCR1_QDIV_RATIO_MASK) >>
> +   DPLL_CFGCR1_QDIV_RATIO_SHIFT;
> +   else
> +   p1 = 1;
> +
> +
> +   switch (p0) {
> +   case DPLL_CFGCR1_PDIV_2:
> +   p0 = 2;
> +   break;
> +   case DPLL_CFGCR1_PDIV_3:
> +   p0 = 3;
> +   break;
> +   case DPLL_CFGCR1_PDIV_5:
> +   p0 = 5;
> +   break;
> +   case DPLL_CFGCR1_PDIV_7:
> +   p0 = 7;
> +   break;
> +   }
> +
> +   switch (p2) {
> +   case DPLL_CFGCR1_KDIV_1:
> +   p2 = 1;
> +   break;
> +   case DPLL_CFGCR1_KDIV_2:
> +   p2 = 2;
> +   break;
> +   case DPLL_CFGCR1_KDIV_4:
> +   p2 = 4;
> +   break;
> +   }
> +
> +   ref_clock = dev_priv->cdclk.hw.ref;
> +
> +   dco_freq = (cfgcr0 & DPLL_CFGCR0_DCO_INTEGER_MASK) * ref_clock;
> +
> +   dco_freq += (((cfgcr0 & DPLL_CFGCR0_DCO_FRACTION_MASK) >>
> + DPLL_CFGCR0_DCO_FRAC_SHIFT) * ref_clock) / 0x8000;
> +
> +   return dco_freq / (p0 * p1 * p2 * 5);
> +}
> +
>  static void ddi_dotclock_get(struct intel_crtc_state *pipe_config)
>  {
> int dotclock;
> @@ -1124,6 +1180,59 @@ static void ddi_dotclock_get(struct intel_crtc_state 
> *pipe_config)
> pipe_config->base.adjusted_mode.crtc_clock = dotclock;
>  }
>
> +static void cnl_ddi_clock_get(struct intel_encoder *encoder,
> + struct intel_crtc_state *pipe_config)
> +{
> +   struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +   int link_clock = 0;
> +   uint32_t cfgcr0, pll_id;
> +
> +   pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
> +
> +   cfgcr0 = I915_READ(CNL_DPLL_CFGCR0(pll_id));
> +
> +   if (cfgcr0 & DPLL_CFGCR0_HDMI_MODE) {
> +   link_clock = cnl_calc_wrpll_link(dev_priv, pll_id);
> +   } else {
> +   link_clock = cfgcr0 & DPLL_CFGCR0_LINK_RATE_MASK;
> +
> +   switch (link_clock) {
> +   case DPLL_CFGCR0_LINK_RATE_810:
> +   link_clock = 81000;
> +   break;
> +   case DPLL_CFGCR0_LINK_RATE_1080:
> +   link_clock = 108000;
> +   break;
> +   case DPLL_CFGCR0_LINK_RATE_1350:
> +   link_clock = 135000;
> + 

[Intel-gfx] [PATCH i-g-t] igt/perf: add tests to verify create/destroy userspace configs

[Lionel Landwerlin]

Signed-off-by: Lionel Landwerlin 
---
 tests/perf.c | 135 +++
 1 file changed, 135 insertions(+)

diff --git a/tests/perf.c b/tests/perf.c
index db28ba1f..14bbb361 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -146,6 +146,33 @@ enum drm_i915_perf_record_type {
 };
 #endif /* !DRM_I915_PERF_OPEN */
 
+#ifndef DRM_IOCTL_I915_PERF_ADD_CONFIG
+
+#define DRM_I915_PERF_ADD_CONFIG   0x37
+#define DRM_I915_PERF_REMOVE_CONFIG0x38
+
+#define DRM_IOCTL_I915_PERF_ADD_CONFIG DRM_IOWR(DRM_COMMAND_BASE + 
DRM_I915_PERF_ADD_CONFIG, struct drm_i915_perf_oa_config)
+#define DRM_IOCTL_I915_PERF_REMOVE_CONFIG  DRM_IOWR(DRM_COMMAND_BASE + 
DRM_I915_PERF_REMOVE_CONFIG, __u64)
+
+/**
+ * Structure to upload perf dynamic configuration into the kernel.
+ */
+struct drm_i915_perf_oa_config {
+   /* string formatted like "%08x-%04x-%04x-%04x-%012x" **/
+   __u64 uuid;
+
+   __u32 n_mux_regs;
+   __u64 mux_regs;
+
+   __u32 n_boolean_regs;
+   __u64 boolean_regs;
+
+   __u32 n_flex_regs;
+   __u64 flex_regs;
+};
+
+#endif /* !DRM_IOCTL_I915_PERF_ADD_CONFIG */
+
 struct accumulator {
 #define MAX_RAW_OA_COUNTERS 62
enum drm_i915_oa_format format;
@@ -4001,6 +4028,108 @@ test_rc6_disable(void)
igt_assert_neq(n_events_end - n_events_start, 0);
 }
 
+static void
+test_create_destroy_userspace_invalid_config(void)
+{
+   struct drm_i915_perf_oa_config userspace_config;
+   const char *uuid = "01234567-0123-0123-0123-0123456789ab";
+   const char *invalid_uuid = "blablabla-wrong";
+   uint32_t mux_regs[] = { 0x9888 /* NOA_WRITE */, 0x0 };
+   uint32_t invalid_mux_regs[] = { 0x12345678 /* invalid register */, 0x0 
};
+
+   memset(_config, 0, sizeof(userspace_config));
+
+   /* invalid uuid */
+   userspace_config.uuid = to_user_pointer(invalid_uuid);
+   userspace_config.n_mux_regs = 1;
+   userspace_config.mux_regs = to_user_pointer(mux_regs);
+   userspace_config.n_boolean_regs = 0;
+   userspace_config.n_flex_regs = 0;
+
+   do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, _config, 
EINVAL);
+
+   /* invalid mux_regs */
+   userspace_config.uuid = to_user_pointer(uuid);
+   userspace_config.n_mux_regs = 1;
+   userspace_config.mux_regs = to_user_pointer(invalid_mux_regs);
+   userspace_config.n_boolean_regs = 0;
+   userspace_config.n_flex_regs = 0;
+
+   do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, _config, 
EINVAL);
+
+   /* empty config */
+   userspace_config.uuid = to_user_pointer(uuid);
+   userspace_config.n_mux_regs = 0;
+   userspace_config.mux_regs = to_user_pointer(mux_regs);
+   userspace_config.n_boolean_regs = 0;
+   userspace_config.n_flex_regs = 0;
+
+   do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, _config, 
EINVAL);
+
+   /* empty config with null pointers */
+   userspace_config.uuid = to_user_pointer(uuid);
+   userspace_config.n_mux_regs = 1;
+   userspace_config.mux_regs = to_user_pointer(NULL);
+   userspace_config.n_boolean_regs = 2;
+   userspace_config.boolean_regs = to_user_pointer(NULL);
+   userspace_config.n_flex_regs = 3;
+   userspace_config.flex_regs = to_user_pointer(NULL);
+
+   do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, _config, 
EINVAL);
+}
+
+static void
+test_create_destroy_userspace_config(void)
+{
+   struct drm_i915_perf_oa_config config;
+   const char *uuid = "01234567-0123-0123-0123-0123456789ab";
+   uint32_t mux_regs[] = { 0x9888 /* NOA_WRITE */, 0x0 };
+   int ret;
+   uint64_t config_id;
+   uint64_t properties[] = {
+   DRM_I915_PERF_PROP_OA_METRICS_SET, 0, /* Filled later */
+
+   /* OA unit configuration */
+   DRM_I915_PERF_PROP_SAMPLE_OA, true,
+   DRM_I915_PERF_PROP_OA_FORMAT, test_oa_format,
+   DRM_I915_PERF_PROP_OA_EXPONENT, oa_exp_1_millisec,
+   DRM_I915_PERF_PROP_OA_METRICS_SET
+   };
+   struct drm_i915_perf_open_param param = {
+   .flags = I915_PERF_FLAG_FD_CLOEXEC |
+   I915_PERF_FLAG_FD_NONBLOCK |
+   I915_PERF_FLAG_DISABLED,
+   .num_properties = ARRAY_SIZE(properties) / 2,
+   .properties_ptr = to_user_pointer(properties),
+   };
+   char path[512];
+
+   snprintf(path, sizeof(path), "/sys/class/drm/card%d/metrics/%s/id", 
card, uuid);
+
+   /* Destroy previous configuration if present */
+   if (try_read_u64_file(path, _id))
+ igt_assert(igt_ioctl(drm_fd, DRM_IOCTL_I915_PERF_REMOVE_CONFIG, 
_id) == 0);
+
+   config.uuid = (uintptr_t) uuid;
+
+   config.n_mux_regs = 1;
+   config.mux_regs = (uintptr_t) mux_regs;
+   config.n_boolean_regs = 0;
+   config.n_flex_regs = 0;
+
+   /* Create a new config */
+   ret = igt_ioctl(drm_fd, 

Re: [Intel-gfx] [PATCH] drm/i915/cnl: Gen10 render context size.

[Rodrigo Vivi]

patch merged to dinq. thanks for reviewing.

On Thu, Jul 6, 2017 at 7:51 PM, Ben Widawsky
 wrote:
> On 17-07-06 14:06:24, Vivi, Rodrigo wrote:
>>
>> No change on render context size is required for Gen10.
>>
>> So this patch doesn't change the default behaviour,
>> but only avoid the missing_case message.
>>
>> Cc: Ben Widawsky 
>> Signed-off-by: Rodrigo Vivi 
>
>
> Reviewed-by: Ben Widawsky 
>
> [snip]
>
>
> --
> Ben Widawsky, Intel Open Source Technology Center
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/cnl: Inherit RPS stuff from previous platforms.

[Rodrigo Vivi]

patch merged to dinq. thanks for reviewing.

On Fri, Jul 7, 2017 at 5:03 AM, David Weinehall
 wrote:
> On Thu, Jul 06, 2017 at 01:41:13PM -0700, Rodrigo Vivi wrote:
>> Apparently no change on RPS stuff from previous platforms.
>>
>> v2: Merging to rps related patches in one and also adding
>> missed cases.
>>
>> Cc: David Weinehall 
>> Signed-off-by: Rodrigo Vivi 
>
> Double-checking BSpec didn't yield anything obvious, so:
>
> Reviewed-by: David Weinehall 
>
>> ---
>>  drivers/gpu/drm/i915/i915_debugfs.c | 20 
>>  drivers/gpu/drm/i915/i915_reg.h |  4 ++--
>>  drivers/gpu/drm/i915/i915_sysfs.c   |  2 +-
>>  drivers/gpu/drm/i915/intel_pm.c | 18 +-
>>  4 files changed, 24 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 643f56b..ca2e34b 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -1159,7 +1159,7 @@ static int i915_frequency_info(struct seq_file *m, 
>> void *unused)
>>   intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>>
>>   reqf = I915_READ(GEN6_RPNSWREQ);
>> - if (IS_GEN9(dev_priv))
>> + if (INTEL_GEN(dev_priv) >= 9)
>>   reqf >>= 23;
>>   else {
>>   reqf &= ~GEN6_TURBO_DISABLE;
>> @@ -1181,7 +1181,7 @@ static int i915_frequency_info(struct seq_file *m, 
>> void *unused)
>>   rpdownei = I915_READ(GEN6_RP_CUR_DOWN_EI) & GEN6_CURIAVG_MASK;
>>   rpcurdown = I915_READ(GEN6_RP_CUR_DOWN) & GEN6_CURBSYTAVG_MASK;
>>   rpprevdown = I915_READ(GEN6_RP_PREV_DOWN) & 
>> GEN6_CURBSYTAVG_MASK;
>> - if (IS_GEN9(dev_priv))
>> + if (INTEL_GEN(dev_priv) >= 9)
>>   cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
>>   else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
>>   cagf = (rpstat & HSW_CAGF_MASK) >> HSW_CAGF_SHIFT;
>> @@ -1210,7 +1210,7 @@ static int i915_frequency_info(struct seq_file *m, 
>> void *unused)
>>  dev_priv->rps.pm_intrmsk_mbz);
>>   seq_printf(m, "GT_PERF_STATUS: 0x%08x\n", gt_perf_status);
>>   seq_printf(m, "Render p-state ratio: %d\n",
>> -(gt_perf_status & (IS_GEN9(dev_priv) ? 0x1ff00 : 
>> 0xff00)) >> 8);
>> +(gt_perf_status & (INTEL_GEN(dev_priv) >= 9 ? 
>> 0x1ff00 : 0xff00)) >> 8);
>>   seq_printf(m, "Render p-state VID: %d\n",
>>  gt_perf_status & 0xff);
>>   seq_printf(m, "Render p-state limit: %d\n",
>> @@ -1241,18 +1241,21 @@ static int i915_frequency_info(struct seq_file *m, 
>> void *unused)
>>
>>   max_freq = (IS_GEN9_LP(dev_priv) ? rp_state_cap >> 0 :
>>   rp_state_cap >> 16) & 0xff;
>> - max_freq *= (IS_GEN9_BC(dev_priv) ? GEN9_FREQ_SCALER : 1);
>> + max_freq *= (IS_GEN9_BC(dev_priv) ||
>> +  IS_CANNONLAKE(dev_priv) ? GEN9_FREQ_SCALER : 1);
>>   seq_printf(m, "Lowest (RPN) frequency: %dMHz\n",
>>  intel_gpu_freq(dev_priv, max_freq));
>>
>>   max_freq = (rp_state_cap & 0xff00) >> 8;
>> - max_freq *= (IS_GEN9_BC(dev_priv) ? GEN9_FREQ_SCALER : 1);
>> + max_freq *= (IS_GEN9_BC(dev_priv) ||
>> +  IS_CANNONLAKE(dev_priv) ? GEN9_FREQ_SCALER : 1);
>>   seq_printf(m, "Nominal (RP1) frequency: %dMHz\n",
>>  intel_gpu_freq(dev_priv, max_freq));
>>
>>   max_freq = (IS_GEN9_LP(dev_priv) ? rp_state_cap >> 16 :
>>   rp_state_cap >> 0) & 0xff;
>> - max_freq *= (IS_GEN9_BC(dev_priv) ? GEN9_FREQ_SCALER : 1);
>> + max_freq *= (IS_GEN9_BC(dev_priv) ||
>> +  IS_CANNONLAKE(dev_priv) ? GEN9_FREQ_SCALER : 1);
>>   seq_printf(m, "Max non-overclocked (RP0) frequency: %dMHz\n",
>>  intel_gpu_freq(dev_priv, max_freq));
>>   seq_printf(m, "Max overclocked frequency: %dMHz\n",
>> @@ -1855,7 +1858,7 @@ static int i915_ring_freq_table(struct seq_file *m, 
>> void *unused)
>>   if (ret)
>>   goto out;
>>
>> - if (IS_GEN9_BC(dev_priv)) {
>> + if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv)) {
>>   /* Convert GT frequency to 50 HZ units */
>>   min_gpu_freq =
>>   dev_priv->rps.min_freq_softlimit / GEN9_FREQ_SCALER;
>> @@ -1875,7 +1878,8 @@ static int i915_ring_freq_table(struct seq_file *m, 
>> void *unused)
>>  _freq);
>>   seq_printf(m, "%d\t\t%d\t\t\t\t%d\n",
>>  

Re: [Intel-gfx] [PATCH] drm/i915/cnl: Don't trust VBT's alternate pin for port D for now.

[Rodrigo Vivi]

patch merged to dinq. thanks for reviewing.

On Thu, Jul 6, 2017 at 2:52 PM, Clint Taylor  wrote:
>
>
> On 07/06/2017 02:08 PM, Rodrigo Vivi wrote:
>>
>> Cannon Lake's VBT that is currently available for B0 stepping
>> states that port D uses alternate pin 3 messing up with the
>> default pin-port mapping table. Using that information we cannot
>> get HDMI working properly. So for now we don't relly on VBT for
>> this information.
>>
>> Cc: Clint Taylor 
>> Signed-off-by: Rodrigo Vivi 
>> ---
>>   drivers/gpu/drm/i915/intel_bios.c | 9 +
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c
>> b/drivers/gpu/drm/i915/intel_bios.c
>> index 639d45c..82b144c 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1187,6 +1187,15 @@ static void parse_ddi_port(struct drm_i915_private
>> *dev_priv, enum port port,
>> if (is_dvi) {
>> info->alternate_ddc_pin = ddc_pin;
>>   + /*
>> +* All VBTs that we got so far for B Stepping has this
>> +* information wrong for Port D. So, let's just ignore for
>> now.
>> +*/
>> +   if (IS_CNL_REVID(dev_priv, CNL_REVID_B0, CNL_REVID_B0) &&
>> +   port == PORT_D) {
>> +   info->alternate_ddc_pin = 0;
>> +   }
>> +
>
>
> Reviewed-by: Clinton Taylor 
>
> -Clint
>
>> sanitize_ddc_pin(dev_priv, port);
>> }
>>
>
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/3] intel: Make driver aware of MOCS table version

[Jason Ekstrand]

On Thu, Jul 6, 2017 at 4:27 PM, Ben Widawsky  wrote:

> We don't yet have optimal MOCS settings, but we have enough to know how
> to at least determine when we might have non-optimal settings within our
> driver.
>
> Signed-off-by: Ben Widawsky 
> ---
>  src/intel/vulkan/anv_device.c | 12 
>  src/intel/vulkan/anv_private.h|  2 ++
>  src/mesa/drivers/dri/i915/intel_context.c |  7 ++-
>  src/mesa/drivers/dri/i965/intel_screen.c  | 14 ++
>  src/mesa/drivers/dri/i965/intel_screen.h  |  2 ++
>  5 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> index 3dc55dbb8d..8e180dbf18 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -368,6 +368,18 @@ anv_physical_device_init(struct anv_physical_device
> *device,
>   device->info.max_cs_threads = max_cs_threads;
> }
>
> +   if (device->info.gen >= 9) {
> +  device->mocs_version = anv_gem_get_param(fd,
> +
>  I915_PARAM_MOCS_TABLE_VERSION);
> +  switch (device->mocs_version) {
> +  default:
> + anv_perf_warn("Kernel exposes newer MOCS table\n");
>

A perf_warn here seems reasonable though it makes more sense to me to make
it

if (device->mocs_version > ANV_MAX_KNOWN_MOCS_VERSION)
   anv_perf_warn("...");


> +  case 1:
> +  case 0:
> + device->mocs_version = MOCS_TABLE_VERSION;
>

Why are we stomping device->mocs_version to MOCS_TABLE_VERSION?  Are you
just trying to avoid the version 0?  If so, why not just have

/* If the MOCS_TABLE_VERSION query fails, assume version 1 */
if (device->mocs_version == 0)
   device->mocs_version = 1;

I don't think we want to have it dependent on a #define in an external
header file.  What if someone updates it for i965 and doesn't update anv or
vice-versa?


> +  }
> +   }
> +
> brw_process_intel_debug_variable();
>
> device->compiler = brw_compiler_create(NULL, >info);
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_
> private.h
> index 573778dad5..b8241a9b22 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -684,6 +684,8 @@ struct anv_physical_device {
>  uint32_teu_total;
>  uint32_tsubslice_total;
>
> +uint8_t mocs_version;
> +
>  struct {
>uint32_t  type_count;
>struct anv_memory_type
> types[VK_MAX_MEMORY_TYPES];
> diff --git a/src/mesa/drivers/dri/i915/intel_context.c
> b/src/mesa/drivers/dri/i915/intel_context.c
> index e0766a0e3f..9169ea650e 100644
> --- a/src/mesa/drivers/dri/i915/intel_context.c
> +++ b/src/mesa/drivers/dri/i915/intel_context.c
> @@ -521,8 +521,13 @@ intelInitContext(struct intel_context *intel,
> INTEL_DEBUG = parse_debug_string(getenv("INTEL_DEBUG"),
> debug_control);
> if (INTEL_DEBUG & DEBUG_BUFMGR)
>dri_bufmgr_set_debug(intel->bufmgr, true);
> -   if (INTEL_DEBUG & DEBUG_PERF)
> +   if (INTEL_DEBUG & DEBUG_PERF) {
>intel->perf_debug = true;
> +  if (screen->mocs_version > MOCS_TABLE_VERSION) {
> + fprintf(stderr, "Kernel exposes newer MOCS table\n");
> + screen->mocs_version = MOCS_TABLE_VERSION;
> +  }
> +   }
>
> if (INTEL_DEBUG & DEBUG_AUB)
>drm_intel_bufmgr_gem_set_aub_dump(intel->bufmgr, true);
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
> b/src/mesa/drivers/dri/i965/intel_screen.c
> index c75f2125d4..c53f133d49 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -2301,6 +2301,20 @@ __DRIconfig **intelInitScreen2(__DRIscreen
> *dri_screen)
>   (ret != -1 || errno != EINVAL);
> }
>
> +   if (devinfo->gen >= 9) {
> +  screen->mocs_version = intel_get_integer(screen,
> +
>  I915_PARAM_MOCS_TABLE_VERSION);
> +  switch (screen->mocs_version) {
> +  case 1:
> +  case 0:
> + screen->mocs_version = MOCS_TABLE_VERSION;
>

Same comments apply here.


> + break;
> +  default:
> + /* We want to perf debug, but we can't yet */
> + break;
> +  }
> +   }
> +
> dri_screen->extensions = !screen->has_context_reset_notification
>? screenExtensions : intelRobustScreenExtensions;
>
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.h
> b/src/mesa/drivers/dri/i965/intel_screen.h
> index f78b3e8f74..eb801f8155 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.h
> +++ b/src/mesa/drivers/dri/i965/intel_screen.h
> @@ -112,6 +112,8 @@ struct intel_screen
> bool mesa_format_supports_texture[MESA_FORMAT_COUNT];
> bool mesa_format_supports_render[MESA_FORMAT_COUNT];
> enum isl_format mesa_to_isl_render_format[MESA_FORMAT_COUNT];
> +
> +   unsigned mocs_version;
>  };
>
>  extern void intelDestroyContext(__DRIcontext 

Re: [Intel-gfx] [Mesa-dev] [PATCH 1/1] drm/i915: Version the MOCS settings

[Jason Ekstrand]

On Fri, Jul 7, 2017 at 3:34 AM, Chris Wilson 
wrote:

> Quoting Ben Widawsky (2017-07-07 00:27:01)
> >  drivers/gpu/drm/i915/i915_drv.c |  3 +++
> >  drivers/gpu/drm/i915/i915_drv.h |  2 ++
>   drivers/gpu/drm/i915/i915_pci.c | 13 +
> >  include/uapi/drm/i915_drm.h |  8 
> >  4 files changed, 22 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c
> > index 9167a73f3c69..26c27b6ae814 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -401,6 +401,9 @@ static int i915_getparam(struct drm_device *dev,
> void *data,
> > if (!value)
> > return -ENODEV;
> > break;
> > +   case I915_PARAM_MOCS_TABLE_VERSION:
> > +   value = INTEL_INFO(dev_priv)->mocs_version;
>
> If we use intel_mocs_get_table_version() we can put this magic number
> in intel_mocs.c next to the tables, where we can keep its history and
> hopefully be able to remember to update it.
>
> > +/* What version of the MOCS table we have. For GEN9 GPUs, the PRM
> defined
> > + * non-optimal settings for the MOCS table. As a result, we were
> required to use a
> > + * small subset, and later add new settings. This param allows
> userspace to
> > + * determine which settings are there.
> > + */
> > +#define MOCS_TABLE_VERSION   1 /* Build time MOCS table
> version */
>
> How are you planing to share this? When we update we bump this number,
> and then mesa copies it across and uses it after verifying it as 0,1 on
> an old kernel.
>

Agreed.  I don't see how having a #define for compile-time mocs version is
useful.  The compile-time version doesn't really matter and we wouldn't
want to use that in i965/anv anyway (more on that in the other patch).


> I don't think you want to expose the updated constant here, but symbolic
> names for each version? (What would be the point?)
>
> Next question, why a version number and not just the number of entries
> defined? Each index is defined by ABI once assigned, so the number of
> entries still operates as a version number and allows easy checking.
>
> if (advanced_cacheing_idx < kernel_max_mocs)
> return advanced_cacheing_idx;
> if (default_cacheing_idx < kernel_max_mocs)
> return default_cacheing_idx;
>
> return follow_pte_idx;
>
> give or take the smarts to choose the preferred indices for any
> particular scenario.
>

I'll have to think about it a bit more but this sounds like a fairly good
idea.  I see two major benefits:

 1. The kernel can return ARRAY_SIZE(mocs_table_for_your_gen) and we will
never forget to update it.
 2. It makes the "does this MOCS value exist" check much easier.  I imagine
future userspace code which chooses mocs values having some sort of "try
and fall back" approach to making MOCS choices and this would be convenient.

That said, having it be a version may have it's advantages, I just don't
know what they are yet.

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

Re: [Intel-gfx] [PATCH v3 21/22] drm/atomic: Introduce drm_atomic_helper_duplicate_commited_state()

[Ville Syrjälä]

On Fri, Jul 07, 2017 at 04:05:28PM +0200, Daniel Vetter wrote:
> On Fri, Jul 7, 2017 at 3:21 PM, Ville Syrjälä
>  wrote:
> > On Fri, Jul 07, 2017 at 02:03:38PM +0200, Daniel Vetter wrote:
> >> On Thu, Jul 06, 2017 at 11:24:41PM +0300, ville.syrj...@linux.intel.com 
> >> wrote:
> >> > From: Ville Syrjälä 
> >> >
> >> > For i915 GPU reset handling we'll want to be able to duplicate the state
> >> > that was last commited to the hardware. For that purpose let's start to
> >> > track the commited state for each object and provide a way to duplicate
> >> > the commmited state into a new drm_atomic_state. The locking for
> >> > .commited_state must to be provided by the driver.
> >> >
> >> > drm_atomic_helper_duplicate_commited_state() duplicates the state
> >> > to both old_state and new_state. For the purposes of i915 GPU reset we
> >> > would only need one of them, but we actually need two top level states;
> >> > one for disabling everything (which would need the duplicated state to
> >> > be old_state), and another to reenable everything (which would need the
> >> > duplicated state to be new_state). So to make it less comples I figured
> >> > I'd just always duplicate both. Might want to rethink this if for no
> >> > other reason that reducing the chances of memory allocation failure.
> >> > Due to the double state duplication we need
> >> > drm_atomic_helper_clean_commited_state() to clean up the duplicated
> >> > old_state since that's not handled by the normal drm_atomic_state
> >> > cleanup code.
> >> >
> >> > TODO: do we want this in the helper, or maybe it should be just in i915?
> >> >
> >> > v2: s/commited/committed/ everywhere (checkpatch)
> >> > Handle state duplication errors better
> >> > v3: Even more care in dealing with memory allocation errors
> >> > Handle private objs too
> >> > Deal with the potential ordering issues between swap_state()
> >> > and hw_done() by keeping track of which state was swapped in
> >> > last
> >> >
> >> > Signed-off-by: Ville Syrjälä 
> >>
> >> I still don't get why we need to duplicate the committed state for gpu
> >> reset. When I said I'm not against adding all that complexity long-term I
> >> meant when we actually really need it. Imo g4x gpu reset isn't a good
> >> justification for that, reworking the atomic world for that seems
> >> massively out of proportion.
> >
> > Well, I still don't see what's so "massive" about a couple of extra state
> > pointers hanging around.
> >
> > Also while doing that state duplication stuff, my old idea of
> > splitting the crtc disable and enable phases into separate atomic
> > commits popped up again in my head. For that being able to duplicate
> > arbitrary states would seem like a nice thing to have. So the
> > refactoring I had to do can have other uses.
> 
> I fully realize that you're unhappy with how atomic ended up getting
> merged and that you think it's a grave mistake. And maybe it is, maybe
> it isn't, I have no idea.

I don't think I ever said that. I've said that it has certain design
problems that we ought to fix. This one being one, and another being
to separate the user state from the internal state. The latter I think
we'll have to tackle rather soon thanks to some new hardware in the
pipeline, or we need to come up with some other way to achieve the
same effect.

> But right now we have _nothing_ asking for
> that reorg afaik, and using gen4 reset to justify it is in my opinion
> simply not solid engineering practice. Maybe we need this in the
> future, and then we can add it, but not before. Refactoring stuff to
> prettify the architecture isn't really useful work.

Neither is having to throw out code that already exists and works. If
you're so worried about time being wasted on pre-g4x GPU reset, then
we could just as well merge my code and move on to more productive
endeavors.

> 
> >> Why exactly can't we do this simpler? I still don't get that part. Is
> >> there really no solution that doesn't break atomic's current assumption
> >> that commits are fully ordered on a given crtc?
> >
> > From the point of view of the old and new states it doesn't actually
> > break that. The commits done from the reset path are essentially
> > invisible to the normal modeset operation.
> 
> You insert something into a fully ordered queue. That does break the
> entire concept and needs a pile of locks and stuff to make it work.

Exactly one lock. Well two if you could the spinlock to protect the
committed_state pointer update from parallel updates touching the same
kms object. That latter one could be removed if atomic wouldn't allow
parallel commits to touch the same object.

> Yes it's doable, but it's a redesign with all the implications of
> subtle breakage all over.

What? It doesn't really even do anything unless you do the
duplicate_committed state(). Everything else is just assigning pointers.
So 

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Unify the HSW/BDW and GEN9+ power well code (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Unify the HSW/BDW and GEN9+ power well code (rev2)
URL   : https://patchwork.freedesktop.org/series/26922/
State : success

== Summary ==

Series 26922v2 drm/i915: Unify the HSW/BDW and GEN9+ power well code
https://patchwork.freedesktop.org/api/1.0/series/26922/revisions/2/mbox/

Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-atomic:
fail   -> PASS   (fi-snb-2600) fdo#100215
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
pass   -> DMESG-WARN (fi-pnv-d510) fdo#101597 +1

fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:444s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:355s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:535s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:511s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:493s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:485s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:600s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:439s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:417s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:425s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:503s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:476s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:466s
fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:593s
fi-kbl-r total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  
time:584s
fi-pnv-d510  total:279  pass:221  dwarn:3   dfail:0   fail:0   skip:55  
time:565s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:462s
fi-skl-6700hqtotal:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  
time:585s
fi-skl-6700k total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  
time:469s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:475s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:434s
fi-skl-x1585ltotal:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:473s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:538s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:405s
fi-bdw-gvtdvm failed to collect. IGT log at Patchwork_5143/fi-bdw-gvtdvm/igt.log

c07f01228c2240ec9604cb9fa4647ccfe575b8a6 drm-tip: 2017y-07m-07d-10h-10m-58s UTC 
integration manifest
adbc9b0 drm/i915: Gather all the power well->domain mappings to one place
07a919f drm/i915: Move hsw_power_well_enable() next to the rest of HSW helpers
43c087e drm/i915/gen9+: Unify the HSW/BDW and GEN9+ power well helpers
d34c37c drm/i915/hsw+: Add has_fuses power well attribute
01897a9 drm/i915/hsw, bdw: Wait for the power well disabled state
f43b6e7 drm/i915/hsw, bdw: Add irq_pipe_mask, has_vga power well attributes
a4fc293 drm/i915/hsw+: Unify the hsw/bdw and gen9+ power well req/state macros
8270740 drm/i915/hsw, bdw: Split power well set to enable/disable helpers
3afa0ea drm/i915/hsw, bdw: Remove redundant state check during power well 
toggling
52856913 drm/i915/gen9+: Remove redundant state check during power well toggling
fb4372a drm/i915/gen9+: Remove redundant power well state assert during enabling
8f01d13 drm/i915/bxt, glk: Give a proper name to the power well struct phy field
ac8523e drm/i915: Check for duplicated power well IDs
d615377 drm/i915/hsw, bdw: Add an ID for the global display power well
4dfaf50 drm/i915/gen2: Add an ID for the display pipes power well
16a60f80 drm/i915: Assign everywhere the always-on power well ID
3f2d8eb drm/i915: Unify power well ID enums
2ad982b drm/i915/chv: Add unique power well ID for the pipe A power well

== Logs ==

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

[Intel-gfx] [PATCH v2 06/18] drm/i915: Check for duplicated power well IDs

[Imre Deak]

Check that all the power well IDs are unique on the given platform.

v2:
- Fix using BIT_ULL() instead of BIT() for 64 bit mask.

Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 27c69f9..bc17d2f 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -2563,6 +2563,8 @@ static uint32_t get_allowed_dc_mask(const struct 
drm_i915_private *dev_priv,
 int intel_power_domains_init(struct drm_i915_private *dev_priv)
 {
struct i915_power_domains *power_domains = _priv->power_domains;
+   u64 power_well_ids;
+   int i;
 
i915.disable_power_well = sanitize_disable_power_well_option(dev_priv,
 i915.disable_power_well);
@@ -2599,6 +2601,15 @@ int intel_power_domains_init(struct drm_i915_private 
*dev_priv)
set_power_wells(power_domains, i9xx_always_on_power_well);
}
 
+   power_well_ids = 0;
+   for (i = 0; i < power_domains->power_well_count; i++) {
+   enum i915_power_well_id id = power_domains->power_wells[i].id;
+
+   WARN_ON(id >= sizeof(power_well_ids) * 8);
+   WARN_ON(power_well_ids & BIT_ULL(id));
+   power_well_ids |= BIT_ULL(id);
+   }
+
return 0;
 }
 
-- 
2.7.4

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

Re: [Intel-gfx] [PATCH v3 21/22] drm/atomic: Introduce drm_atomic_helper_duplicate_commited_state()

[Daniel Vetter]

On Fri, Jul 7, 2017 at 3:21 PM, Ville Syrjälä
 wrote:
> On Fri, Jul 07, 2017 at 02:03:38PM +0200, Daniel Vetter wrote:
>> On Thu, Jul 06, 2017 at 11:24:41PM +0300, ville.syrj...@linux.intel.com 
>> wrote:
>> > From: Ville Syrjälä 
>> >
>> > For i915 GPU reset handling we'll want to be able to duplicate the state
>> > that was last commited to the hardware. For that purpose let's start to
>> > track the commited state for each object and provide a way to duplicate
>> > the commmited state into a new drm_atomic_state. The locking for
>> > .commited_state must to be provided by the driver.
>> >
>> > drm_atomic_helper_duplicate_commited_state() duplicates the state
>> > to both old_state and new_state. For the purposes of i915 GPU reset we
>> > would only need one of them, but we actually need two top level states;
>> > one for disabling everything (which would need the duplicated state to
>> > be old_state), and another to reenable everything (which would need the
>> > duplicated state to be new_state). So to make it less comples I figured
>> > I'd just always duplicate both. Might want to rethink this if for no
>> > other reason that reducing the chances of memory allocation failure.
>> > Due to the double state duplication we need
>> > drm_atomic_helper_clean_commited_state() to clean up the duplicated
>> > old_state since that's not handled by the normal drm_atomic_state
>> > cleanup code.
>> >
>> > TODO: do we want this in the helper, or maybe it should be just in i915?
>> >
>> > v2: s/commited/committed/ everywhere (checkpatch)
>> > Handle state duplication errors better
>> > v3: Even more care in dealing with memory allocation errors
>> > Handle private objs too
>> > Deal with the potential ordering issues between swap_state()
>> > and hw_done() by keeping track of which state was swapped in
>> > last
>> >
>> > Signed-off-by: Ville Syrjälä 
>>
>> I still don't get why we need to duplicate the committed state for gpu
>> reset. When I said I'm not against adding all that complexity long-term I
>> meant when we actually really need it. Imo g4x gpu reset isn't a good
>> justification for that, reworking the atomic world for that seems
>> massively out of proportion.
>
> Well, I still don't see what's so "massive" about a couple of extra state
> pointers hanging around.
>
> Also while doing that state duplication stuff, my old idea of
> splitting the crtc disable and enable phases into separate atomic
> commits popped up again in my head. For that being able to duplicate
> arbitrary states would seem like a nice thing to have. So the
> refactoring I had to do can have other uses.

I fully realize that you're unhappy with how atomic ended up getting
merged and that you think it's a grave mistake. And maybe it is, maybe
it isn't, I have no idea. But right now we have _nothing_ asking for
that reorg afaik, and using gen4 reset to justify it is in my opinion
simply not solid engineering practice. Maybe we need this in the
future, and then we can add it, but not before. Refactoring stuff to
prettify the architecture isn't really useful work.

>> Why exactly can't we do this simpler? I still don't get that part. Is
>> there really no solution that doesn't break atomic's current assumption
>> that commits are fully ordered on a given crtc?
>
> From the point of view of the old and new states it doesn't actually
> break that. The commits done from the reset path are essentially
> invisible to the normal modeset operation.

You insert something into a fully ordered queue. That does break the
entire concept and needs a pile of locks and stuff to make it work.
Yes it's doable, but it's a redesign with all the implications of
subtle breakage all over. While we do have open bugs for the current
design. Rewriting the world to fix a bug needs seriously better
justification imo.

> The one alternative proposed idea of allowing gem and kms sides go
> out of whack scares me a bit. I think that might land us in more
> trouble when I finally get around to making the video overlay a
> drm_plane.

We've run perfectly fine with this idea for years.

> And I think trying to keep the GPU reset paths as similar as possible
> between all the platforms would be a nice thing. Just whacking
> everything on the head with a hammer on one platform but not on
> another one seems to me like extra variation in behaviour that we
> don't necessarily want.
>
> But like I said, if someone can come up with a better solution I
> probably wouldn't object too much. It's not going to be coming from me
> though since I have plenty of other things to do and vacation time is
> coming up very soon. So unless someone else comes up with something nice
> soon I think we should just go with my solution because a) it's already
> available, and b) works quite decently from what I can see.

I guess I'll have to retype the old thing in the new world, 

Re: [Intel-gfx] [PATCH IGT v2 2/6] lib: Add PSR utility functions to igt library.

[Jim Bride]

On Fri, Jun 30, 2017 at 05:54:32PM -0300, Paulo Zanoni wrote:
> Em Sex, 2017-06-30 às 12:12 -0700, Jim Bride escreveu:
> > Factor out some code that was replicated in three test utilities into
> > some new IGT library functions so that we are checking PSR status in
> > a consistent fashion across all of our PSR tests.
> > 
> > v2: * Add sink CRC helper function
> > * Misc. bug fixes
> > * Rebase
> > 
> > Cc: Rodrigo Vivi 
> > Cc: Paulo Zanoni 
> > Signed-off-by: Jim Bride 
> > ---
> >  lib/Makefile.sources |   2 +
> >  lib/igt.h|   1 +
> >  lib/igt_psr.c| 235
> > +++
> >  lib/igt_psr.h|  43 ++
> >  4 files changed, 281 insertions(+)
> >  create mode 100644 lib/igt_psr.c
> >  create mode 100644 lib/igt_psr.h
> > 
> > diff --git a/lib/Makefile.sources b/lib/Makefile.sources
> > index 53fdb54..6a73c8c 100644
> > --- a/lib/Makefile.sources
> > +++ b/lib/Makefile.sources
> > @@ -83,6 +83,8 @@ lib_source_list =     \
> >     uwildmat/uwildmat.c \
> >     igt_kmod.c  \
> >     igt_kmod.h  \
> > +   igt_psr.c   \
> > +   igt_psr.h   \
> >     $(NULL)
> >  
> >  .PHONY: version.h.tmp
> > diff --git a/lib/igt.h b/lib/igt.h
> > index a069deb..0b8e3a8 100644
> > --- a/lib/igt.h
> > +++ b/lib/igt.h
> > @@ -37,6 +37,7 @@
> >  #include "igt_gt.h"
> >  #include "igt_kms.h"
> >  #include "igt_pm.h"
> > +#include "igt_psr.h"
> >  #include "igt_stats.h"
> >  #ifdef HAVE_CHAMELIUM
> >  #include "igt_chamelium.h"
> > diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> > new file mode 100644
> > index 000..a2823bf
> > --- /dev/null
> > +++ b/lib/igt_psr.c
> > @@ -0,0 +1,235 @@
> > +/*
> > + * Copyright © 2017 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.
> > + */
> > +
> > +#include "igt.h"
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/**
> > + * SECTION:igt_psr
> > + * @short_description: Panel Self Refresh helpers
> > + * @title: Panel Self Refresh
> > + * @include: igt.h
> > + *
> > + * This library provides various helpers to enable Panel Self
> > Refresh,
> > + * as well as to check the state of PSR on the system (enabled vs.
> > + * disabled, active vs. inactive) or to wait for PSR to be active
> > + * or inactive.
> > + */
> > +
> > +/**
> > + * igt_psr_source_support:
> > + *
> > + * Returns true if the source supports PSR.
> > + */
> > +bool igt_psr_source_support(int fd)
> > +{
> > +   char buf[512];
> > +
> > +   igt_debugfs_read(fd, "i915_edp_psr_status", buf);
> > +
> > +   return strstr(buf, "Source_OK: yes\n");
> > +}
> > +
> > +
> > +/**
> > + * igt_psr_sink_support:
> > + *
> > + * Returns true if the current eDP panel supports PSR.
> > + */
> > +bool igt_psr_sink_support(int fd)
> > +{
> > +   char buf[256];
> 
> Why are some buffers 512 while the others are 256? They're both for the
> same file.

It's arbitrary.  I'll make things consistent when I revisit this patch.

> > +
> > +   igt_debugfs_read(fd, "i915_edp_psr_status", buf);
> > +   return strstr(buf, "Sink_Support: yes\n");
> > +}
> > +
> > +/**
> > + * igt_psr_possible:
> > + *
> > + * Returns true if both the source and sink support PSR.
> > + */
> > +bool igt_psr_possible(int fd)
> > +{
> > +   char buf[512];
> > +
> > +   igt_debugfs_read(fd, "i915_edp_psr_status", buf);
> > +
> > +   return igt_psr_source_support(fd) &&
> > igt_psr_sink_support(fd);
> > +}
> > +
> > +/**
> > + * igt_psr_active:
> > + *
> > + * Returns true if PSR is active on the panel.
> > + */
> > +bool igt_psr_active(int fd)
> > +{
> > +   char buf[512];
> > +   bool actret = false;
> > +   bool hwactret = 

Re: [Intel-gfx] [PATCH IGT v2 2/6] lib: Add PSR utility functions to igt library.

[Jim Bride]

On Fri, Jun 30, 2017 at 01:11:52PM -0700, Rodrigo Vivi wrote:
> On Fri, Jun 30, 2017 at 12:12 PM, Jim Bride  wrote:
> > Factor out some code that was replicated in three test utilities into
> > some new IGT library functions so that we are checking PSR status in
> > a consistent fashion across all of our PSR tests.
> >
> > v2: * Add sink CRC helper function
> > * Misc. bug fixes
> > * Rebase
> >
> > Cc: Rodrigo Vivi 
> > Cc: Paulo Zanoni 
> > Signed-off-by: Jim Bride 
> > ---
> >  lib/Makefile.sources |   2 +
> >  lib/igt.h|   1 +
> >  lib/igt_psr.c| 235 
> > +++
> >  lib/igt_psr.h|  43 ++
> >  4 files changed, 281 insertions(+)
> >  create mode 100644 lib/igt_psr.c
> >  create mode 100644 lib/igt_psr.h
> >
> > diff --git a/lib/Makefile.sources b/lib/Makefile.sources
> > index 53fdb54..6a73c8c 100644
> > --- a/lib/Makefile.sources
> > +++ b/lib/Makefile.sources
> > @@ -83,6 +83,8 @@ lib_source_list = \
> > uwildmat/uwildmat.c \
> > igt_kmod.c  \
> > igt_kmod.h  \
> > +   igt_psr.c   \
> > +   igt_psr.h   \
> > $(NULL)
> >
> >  .PHONY: version.h.tmp
> > diff --git a/lib/igt.h b/lib/igt.h
> > index a069deb..0b8e3a8 100644
> > --- a/lib/igt.h
> > +++ b/lib/igt.h
> > @@ -37,6 +37,7 @@
> >  #include "igt_gt.h"
> >  #include "igt_kms.h"
> >  #include "igt_pm.h"
> > +#include "igt_psr.h"
> >  #include "igt_stats.h"
> >  #ifdef HAVE_CHAMELIUM
> >  #include "igt_chamelium.h"
> > diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> > new file mode 100644
> > index 000..a2823bf
> > --- /dev/null
> > +++ b/lib/igt_psr.c
> > @@ -0,0 +1,235 @@
> > +/*
> > + * Copyright © 2017 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.
> > + */
> > +
> > +#include "igt.h"
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/**
> > + * SECTION:igt_psr
> > + * @short_description: Panel Self Refresh helpers
> > + * @title: Panel Self Refresh
> > + * @include: igt.h
> > + *
> > + * This library provides various helpers to enable Panel Self Refresh,
> > + * as well as to check the state of PSR on the system (enabled vs.
> > + * disabled, active vs. inactive) or to wait for PSR to be active
> > + * or inactive.
> > + */
> > +
> > +/**
> > + * igt_psr_source_support:
> > + *
> > + * Returns true if the source supports PSR.
> > + */
> > +bool igt_psr_source_support(int fd)
> > +{
> > +   char buf[512];
> > +
> > +   igt_debugfs_read(fd, "i915_edp_psr_status", buf);
> > +
> > +   return strstr(buf, "Source_OK: yes\n");
> > +}
> > +
> > +
> > +/**
> > + * igt_psr_sink_support:
> > + *
> > + * Returns true if the current eDP panel supports PSR.
> > + */
> > +bool igt_psr_sink_support(int fd)
> > +{
> > +   char buf[256];
> > +
> > +   igt_debugfs_read(fd, "i915_edp_psr_status", buf);
> > +   return strstr(buf, "Sink_Support: yes\n");
> > +}
> > +
> > +/**
> > + * igt_psr_possible:
> > + *
> > + * Returns true if both the source and sink support PSR.
> > + */
> > +bool igt_psr_possible(int fd)
> > +{
> > +   char buf[512];
> > +
> > +   igt_debugfs_read(fd, "i915_edp_psr_status", buf);
> > +
> > +   return igt_psr_source_support(fd) && igt_psr_sink_support(fd);
> > +}
> > +
> > +/**
> > + * igt_psr_active:
> > + *
> > + * Returns true if PSR is active on the panel.
> > + */
> > +bool igt_psr_active(int fd)
> > +{
> > +   char buf[512];
> > +   bool actret = false;
> > +   bool hwactret = false;
> > +
> > +   igt_debugfs_read(fd, "i915_edp_psr_status", buf);
> > +   hwactret = 

Re: [Intel-gfx] [PATCH 5/5] drm/hisilicon: Remove custom FB helper deferred setup

[Sean Paul]

On Thu, Jul 6, 2017 at 9:00 AM, Daniel Vetter  wrote:
> From: Thierry Reding 
>
> The FB helper core now supports deferred setup, so the driver's custom
> implementation can be removed.
>
> v2: Dont' resurrect drm_vblank_cleanup.
>
> Cc: Xinliang Liu 
> Cc: Rongrong Zou 
> Cc: Xinwei Kong 
> Cc: Chen Feng 
> Signed-off-by: Thierry Reding  (v1)
> Signed-off-by: Daniel Vetter 

Reviewed-by: Sean Paul 

> ---
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c 
> b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> index 8065d6cb1d7f..1178341c3858 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> @@ -54,14 +54,7 @@ static void kirin_fbdev_output_poll_changed(struct 
> drm_device *dev)
>  {
> struct kirin_drm_private *priv = dev->dev_private;
>
> -   if (priv->fbdev) {
> -   drm_fbdev_cma_hotplug_event(priv->fbdev);
> -   } else {
> -   priv->fbdev = drm_fbdev_cma_init(dev, 32,
> -
> dev->mode_config.num_connector);
> -   if (IS_ERR(priv->fbdev))
> -   priv->fbdev = NULL;
> -   }
> +   drm_fbdev_cma_hotplug_event(priv->fbdev);
>  }
>  #endif
>
> @@ -128,11 +121,18 @@ static int kirin_drm_kms_init(struct drm_device *dev)
> /* init kms poll for handling hpd */
> drm_kms_helper_poll_init(dev);
>
> -   /* force detection after connectors init */
> -   (void)drm_helper_hpd_irq_event(dev);
> +   priv->fbdev = drm_fbdev_cma_init(dev, 32,
> +dev->mode_config.num_connector);
> +   if (IS_ERR(priv->fbdev)) {
> +   DRM_ERROR("failed to initialize fbdev.\n");
> +   ret = PTR_ERR(priv->fbdev);
> +   goto err_cleanup_poll;
> +   }
>
> return 0;
>
> +err_cleanup_poll:
> +   drm_kms_helper_poll_fini(dev);
>  err_unbind_all:
> component_unbind_all(dev->dev, dev);
>  err_dc_cleanup:
> --
> 2.13.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3 21/22] drm/atomic: Introduce drm_atomic_helper_duplicate_commited_state()

[Ville Syrjälä]

On Fri, Jul 07, 2017 at 02:03:38PM +0200, Daniel Vetter wrote:
> On Thu, Jul 06, 2017 at 11:24:41PM +0300, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> > 
> > For i915 GPU reset handling we'll want to be able to duplicate the state
> > that was last commited to the hardware. For that purpose let's start to
> > track the commited state for each object and provide a way to duplicate
> > the commmited state into a new drm_atomic_state. The locking for
> > .commited_state must to be provided by the driver.
> > 
> > drm_atomic_helper_duplicate_commited_state() duplicates the state
> > to both old_state and new_state. For the purposes of i915 GPU reset we
> > would only need one of them, but we actually need two top level states;
> > one for disabling everything (which would need the duplicated state to
> > be old_state), and another to reenable everything (which would need the
> > duplicated state to be new_state). So to make it less comples I figured
> > I'd just always duplicate both. Might want to rethink this if for no
> > other reason that reducing the chances of memory allocation failure.
> > Due to the double state duplication we need
> > drm_atomic_helper_clean_commited_state() to clean up the duplicated
> > old_state since that's not handled by the normal drm_atomic_state
> > cleanup code.
> > 
> > TODO: do we want this in the helper, or maybe it should be just in i915?
> > 
> > v2: s/commited/committed/ everywhere (checkpatch)
> > Handle state duplication errors better
> > v3: Even more care in dealing with memory allocation errors
> > Handle private objs too
> > Deal with the potential ordering issues between swap_state()
> > and hw_done() by keeping track of which state was swapped in
> > last
> > 
> > Signed-off-by: Ville Syrjälä 
> 
> I still don't get why we need to duplicate the committed state for gpu
> reset. When I said I'm not against adding all that complexity long-term I
> meant when we actually really need it. Imo g4x gpu reset isn't a good
> justification for that, reworking the atomic world for that seems
> massively out of proportion.

Well, I still don't see what's so "massive" about a couple of extra state
pointers hanging around.

Also while doing that state duplication stuff, my old idea of
splitting the crtc disable and enable phases into separate atomic
commits popped up again in my head. For that being able to duplicate
arbitrary states would seem like a nice thing to have. So the
refactoring I had to do can have other uses.

> Why exactly can't we do this simpler? I still don't get that part. Is
> there really no solution that doesn't break atomic's current assumption
> that commits are fully ordered on a given crtc?

From the point of view of the old and new states it doesn't actually
break that. The commits done from the reset path are essentially
invisible to the normal modeset operation.

The one alternative proposed idea of allowing gem and kms sides go
out of whack scares me a bit. I think that might land us in more
trouble when I finally get around to making the video overlay a
drm_plane.

And I think trying to keep the GPU reset paths as similar as possible
between all the platforms would be a nice thing. Just whacking
everything on the head with a hammer on one platform but not on
another one seems to me like extra variation in behaviour that we
don't necessarily want.

But like I said, if someone can come up with a better solution I
probably wouldn't object too much. It's not going to be coming from me
though since I have plenty of other things to do and vacation time is
coming up very soon. So unless someone else comes up with something nice
soon I think we should just go with my solution because a) it's already
available, and b) works quite decently from what I can see.

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/drm_atomic.c|   1 +
> >  drivers/gpu/drm/drm_atomic_helper.c | 231 
> > 
> >  include/drm/drm_atomic.h|   4 +
> >  include/drm/drm_atomic_helper.h |   8 ++
> >  include/drm/drm_connector.h |  11 ++
> >  include/drm/drm_crtc.h  |  11 ++
> >  include/drm/drm_plane.h |  11 ++
> >  7 files changed, 277 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 56925b93f598..e1578d50d66f 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -1020,6 +1020,7 @@ drm_atomic_private_obj_init(struct drm_private_obj 
> > *obj,
> > memset(obj, 0, sizeof(*obj));
> >  
> > obj->state = state;
> > +   obj->committed_state = state;
> > obj->funcs = funcs;
> >  }
> >  EXPORT_SYMBOL(drm_atomic_private_obj_init);
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index f0887f231fb8..c3d02f12cd5d 100644
> > --- 

Re: [Intel-gfx] [PATCH] drm/i915/cnl: Inherit RPS stuff from previous platforms.

[David Weinehall]

On Thu, Jul 06, 2017 at 01:41:13PM -0700, Rodrigo Vivi wrote:
> Apparently no change on RPS stuff from previous platforms.
> 
> v2: Merging to rps related patches in one and also adding
> missed cases.
> 
> Cc: David Weinehall 
> Signed-off-by: Rodrigo Vivi 

Double-checking BSpec didn't yield anything obvious, so:

Reviewed-by: David Weinehall 

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 20 
>  drivers/gpu/drm/i915/i915_reg.h |  4 ++--
>  drivers/gpu/drm/i915/i915_sysfs.c   |  2 +-
>  drivers/gpu/drm/i915/intel_pm.c | 18 +-
>  4 files changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 643f56b..ca2e34b 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1159,7 +1159,7 @@ static int i915_frequency_info(struct seq_file *m, void 
> *unused)
>   intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>  
>   reqf = I915_READ(GEN6_RPNSWREQ);
> - if (IS_GEN9(dev_priv))
> + if (INTEL_GEN(dev_priv) >= 9)
>   reqf >>= 23;
>   else {
>   reqf &= ~GEN6_TURBO_DISABLE;
> @@ -1181,7 +1181,7 @@ static int i915_frequency_info(struct seq_file *m, void 
> *unused)
>   rpdownei = I915_READ(GEN6_RP_CUR_DOWN_EI) & GEN6_CURIAVG_MASK;
>   rpcurdown = I915_READ(GEN6_RP_CUR_DOWN) & GEN6_CURBSYTAVG_MASK;
>   rpprevdown = I915_READ(GEN6_RP_PREV_DOWN) & 
> GEN6_CURBSYTAVG_MASK;
> - if (IS_GEN9(dev_priv))
> + if (INTEL_GEN(dev_priv) >= 9)
>   cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
>   else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
>   cagf = (rpstat & HSW_CAGF_MASK) >> HSW_CAGF_SHIFT;
> @@ -1210,7 +1210,7 @@ static int i915_frequency_info(struct seq_file *m, void 
> *unused)
>  dev_priv->rps.pm_intrmsk_mbz);
>   seq_printf(m, "GT_PERF_STATUS: 0x%08x\n", gt_perf_status);
>   seq_printf(m, "Render p-state ratio: %d\n",
> -(gt_perf_status & (IS_GEN9(dev_priv) ? 0x1ff00 : 
> 0xff00)) >> 8);
> +(gt_perf_status & (INTEL_GEN(dev_priv) >= 9 ? 
> 0x1ff00 : 0xff00)) >> 8);
>   seq_printf(m, "Render p-state VID: %d\n",
>  gt_perf_status & 0xff);
>   seq_printf(m, "Render p-state limit: %d\n",
> @@ -1241,18 +1241,21 @@ static int i915_frequency_info(struct seq_file *m, 
> void *unused)
>  
>   max_freq = (IS_GEN9_LP(dev_priv) ? rp_state_cap >> 0 :
>   rp_state_cap >> 16) & 0xff;
> - max_freq *= (IS_GEN9_BC(dev_priv) ? GEN9_FREQ_SCALER : 1);
> + max_freq *= (IS_GEN9_BC(dev_priv) ||
> +  IS_CANNONLAKE(dev_priv) ? GEN9_FREQ_SCALER : 1);
>   seq_printf(m, "Lowest (RPN) frequency: %dMHz\n",
>  intel_gpu_freq(dev_priv, max_freq));
>  
>   max_freq = (rp_state_cap & 0xff00) >> 8;
> - max_freq *= (IS_GEN9_BC(dev_priv) ? GEN9_FREQ_SCALER : 1);
> + max_freq *= (IS_GEN9_BC(dev_priv) ||
> +  IS_CANNONLAKE(dev_priv) ? GEN9_FREQ_SCALER : 1);
>   seq_printf(m, "Nominal (RP1) frequency: %dMHz\n",
>  intel_gpu_freq(dev_priv, max_freq));
>  
>   max_freq = (IS_GEN9_LP(dev_priv) ? rp_state_cap >> 16 :
>   rp_state_cap >> 0) & 0xff;
> - max_freq *= (IS_GEN9_BC(dev_priv) ? GEN9_FREQ_SCALER : 1);
> + max_freq *= (IS_GEN9_BC(dev_priv) ||
> +  IS_CANNONLAKE(dev_priv) ? GEN9_FREQ_SCALER : 1);
>   seq_printf(m, "Max non-overclocked (RP0) frequency: %dMHz\n",
>  intel_gpu_freq(dev_priv, max_freq));
>   seq_printf(m, "Max overclocked frequency: %dMHz\n",
> @@ -1855,7 +1858,7 @@ static int i915_ring_freq_table(struct seq_file *m, 
> void *unused)
>   if (ret)
>   goto out;
>  
> - if (IS_GEN9_BC(dev_priv)) {
> + if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv)) {
>   /* Convert GT frequency to 50 HZ units */
>   min_gpu_freq =
>   dev_priv->rps.min_freq_softlimit / GEN9_FREQ_SCALER;
> @@ -1875,7 +1878,8 @@ static int i915_ring_freq_table(struct seq_file *m, 
> void *unused)
>  _freq);
>   seq_printf(m, "%d\t\t%d\t\t\t\t%d\n",
>  intel_gpu_freq(dev_priv, (gpu_freq *
> -  (IS_GEN9_BC(dev_priv) ?
> +  (IS_GEN9_BC(dev_priv) ||
> + 

Re: [Intel-gfx] [PATCH v3 21/22] drm/atomic: Introduce drm_atomic_helper_duplicate_commited_state()

[Daniel Vetter]

On Thu, Jul 06, 2017 at 11:24:41PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> For i915 GPU reset handling we'll want to be able to duplicate the state
> that was last commited to the hardware. For that purpose let's start to
> track the commited state for each object and provide a way to duplicate
> the commmited state into a new drm_atomic_state. The locking for
> .commited_state must to be provided by the driver.
> 
> drm_atomic_helper_duplicate_commited_state() duplicates the state
> to both old_state and new_state. For the purposes of i915 GPU reset we
> would only need one of them, but we actually need two top level states;
> one for disabling everything (which would need the duplicated state to
> be old_state), and another to reenable everything (which would need the
> duplicated state to be new_state). So to make it less comples I figured
> I'd just always duplicate both. Might want to rethink this if for no
> other reason that reducing the chances of memory allocation failure.
> Due to the double state duplication we need
> drm_atomic_helper_clean_commited_state() to clean up the duplicated
> old_state since that's not handled by the normal drm_atomic_state
> cleanup code.
> 
> TODO: do we want this in the helper, or maybe it should be just in i915?
> 
> v2: s/commited/committed/ everywhere (checkpatch)
> Handle state duplication errors better
> v3: Even more care in dealing with memory allocation errors
> Handle private objs too
> Deal with the potential ordering issues between swap_state()
> and hw_done() by keeping track of which state was swapped in
> last
> 
> Signed-off-by: Ville Syrjälä 

I still don't get why we need to duplicate the committed state for gpu
reset. When I said I'm not against adding all that complexity long-term I
meant when we actually really need it. Imo g4x gpu reset isn't a good
justification for that, reworking the atomic world for that seems
massively out of proportion.

Why exactly can't we do this simpler? I still don't get that part. Is
there really no solution that doesn't break atomic's current assumption
that commits are fully ordered on a given crtc?
-Daniel

> ---
>  drivers/gpu/drm/drm_atomic.c|   1 +
>  drivers/gpu/drm/drm_atomic_helper.c | 231 
> 
>  include/drm/drm_atomic.h|   4 +
>  include/drm/drm_atomic_helper.h |   8 ++
>  include/drm/drm_connector.h |  11 ++
>  include/drm/drm_crtc.h  |  11 ++
>  include/drm/drm_plane.h |  11 ++
>  7 files changed, 277 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 56925b93f598..e1578d50d66f 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1020,6 +1020,7 @@ drm_atomic_private_obj_init(struct drm_private_obj *obj,
>   memset(obj, 0, sizeof(*obj));
>  
>   obj->state = state;
> + obj->committed_state = state;
>   obj->funcs = funcs;
>  }
>  EXPORT_SYMBOL(drm_atomic_private_obj_init);
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index f0887f231fb8..c3d02f12cd5d 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1815,6 +1815,11 @@ void drm_atomic_helper_wait_for_dependencies(struct 
> drm_atomic_state *old_state)
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_wait_for_dependencies);
>  
> +static bool state_seqno_after(unsigned int a, unsigned int b)
> +{
> + return (int)(b - a) < 0;
> +}
> +
>  /**
>   * drm_atomic_helper_commit_hw_done - setup possible nonblocking commit
>   * @old_state: atomic state object with old state structures
> @@ -1833,11 +1838,39 @@ 
> EXPORT_SYMBOL(drm_atomic_helper_wait_for_dependencies);
>  void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_state)
>  {
>   struct drm_crtc *crtc;
> + struct drm_plane *plane;
> + struct drm_connector *connector;
> + struct drm_private_obj *obj;
>   struct drm_crtc_state *new_crtc_state;
> + struct drm_plane_state *new_plane_state;
> + struct drm_connector_state *new_connector_state;
> + struct drm_private_state *new_obj_state;
>   struct drm_crtc_commit *commit;
>   int i;
> + static DEFINE_SPINLOCK(committed_state_lock);
> +
> + spin_lock(_state_lock);
> +
> + for_each_new_plane_in_state(old_state, plane, new_plane_state, i) {
> + if (plane->committed_state &&
> + state_seqno_after(new_plane_state->seqno,
> +   plane->committed_state->seqno))
> + plane->committed_state = new_plane_state;
> + }
> +
> + for_each_new_connector_in_state(old_state, connector, 
> new_connector_state, i) {
> + if (connector->committed_state &&
> + state_seqno_after(new_connector_state->seqno,
> +

Re: [Intel-gfx] [PATCH 13/15] drm/i915: Allow execbuffer to use the first object as the batch

[Chris Wilson]

Quoting Daniel Vetter (2017-07-07 11:17:22)
> On Fri, Mar 17, 2017 at 12:15 PM, Joonas Lahtinen
>  wrote:
> > On to, 2017-03-16 at 13:20 +, Chris Wilson wrote:
> >> Currently, the last object in the execlist is the always the batch.
> >> However, when building the batch buffer we often know the batch object
> >> first and if we can use the first slot in the execlist we can emit
> >> relocation instructions relative to it immediately and avoid a separate
> >> pass to adjust the relocations to point to the last execlist slot.
> >>
> >> Signed-off-by: Chris Wilson 
> >
> > Reviewed-by: Joonas Lahtinen 
> 
> This patch was reviewed/pushed full month before the mesa patch was
> fully reviewed and ready for merging. That's not how uapi is done.
> 
> I've fixed this up now by at least reviewing the mesa patch, but for
> next time around: If you review uapi, and you don't make sure the
> userspace side is in good shape too, then you've not reviewed the
> patch properly.

The userspace was in good shape and had been requested. Now explain the
complication in the API that merits a rant?

You've had well over 2 years to review the patches that first used it.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Fix up CNL cdclk related limits

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Fix up CNL cdclk related limits
URL   : https://patchwork.freedesktop.org/series/26988/
State : success

== Summary ==

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

Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass   -> DMESG-WARN (fi-kbl-7560u) fdo#100125
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-atomic:
fail   -> PASS   (fi-snb-2600) fdo#100215
Test kms_flip:
Subgroup basic-flip-vs-modeset:
skip   -> PASS   (fi-skl-x1585l)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> PASS   (fi-byt-n2820) fdo#101705

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:442s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:427s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:350s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:531s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:513s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:489s
fi-byt-n2820 total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:478s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:593s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:433s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:419s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:419s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:484s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:470s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:460s
fi-kbl-7560u total:279  pass:268  dwarn:1   dfail:0   fail:0   skip:10  
time:572s
fi-kbl-r total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  
time:584s
fi-pnv-d510  total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  
time:561s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:460s
fi-skl-6700hqtotal:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  
time:585s
fi-skl-6700k total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  
time:467s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:481s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:439s
fi-skl-x1585ltotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:488s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:540s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:407s

c07f01228c2240ec9604cb9fa4647ccfe575b8a6 drm-tip: 2017y-07m-07d-10h-10m-58s UTC 
integration manifest
565b523 drm/i915: Consolidate max_cdclk_freq check in 
intel_crtc_compute_min_cdclk()
78592a4b drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock"
11d4ee5 drm/i915: Fix up CNL cdclk related limits

== Logs ==

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

Re: [Intel-gfx] [Mesa-dev] [PATCH 1/1] drm/i915: Version the MOCS settings

[Emil Velikov]

On 7 July 2017 at 11:34, Chris Wilson  wrote:
> Quoting Ben Widawsky (2017-07-07 00:27:01)
>>  drivers/gpu/drm/i915/i915_drv.c |  3 +++
>>  drivers/gpu/drm/i915/i915_drv.h |  2 ++
>>  drivers/gpu/drm/i915/i915_pci.c | 13 +
>>  include/uapi/drm/i915_drm.h |  8 
>>  4 files changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c 
>> b/drivers/gpu/drm/i915/i915_drv.c
>> index 9167a73f3c69..26c27b6ae814 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -401,6 +401,9 @@ static int i915_getparam(struct drm_device *dev, void 
>> *data,
>> if (!value)
>> return -ENODEV;
>> break;
>> +   case I915_PARAM_MOCS_TABLE_VERSION:
>> +   value = INTEL_INFO(dev_priv)->mocs_version;
>
> If we use intel_mocs_get_table_version() we can put this magic number
> in intel_mocs.c next to the tables, where we can keep its history and
> hopefully be able to remember to update it.
>
>> +/* What version of the MOCS table we have. For GEN9 GPUs, the PRM defined
>> + * non-optimal settings for the MOCS table. As a result, we were required 
>> to use a
>> + * small subset, and later add new settings. This param allows userspace to
>> + * determine which settings are there.
>> + */
>> +#define MOCS_TABLE_VERSION   1 /* Build time MOCS table version 
>> */
>
> How are you planing to share this? When we update we bump this number,
> and then mesa copies it across and uses it after verifying it as 0,1 on
> an old kernel.
>
> I don't think you want to expose the updated constant here, but symbolic
> names for each version? (What would be the point?)
>
FWIW I have to agree with Chris here - having the value is of limited
use. Furthermore it mostly confuses people when writing the user space
parts.

For example:
Mesa implements v1 and uses the define. Kernel headers get updated to
v2 and Mesa supporting v1 gets rebuild against them.
Mesa stores/treats as the MOCS version has "v2" when the actual
hardware/kernel supports "v1".

The expected issues vary depending on the implementation, but I
suspect it won't be fun :-)

IMHO it's better if user space is explicit on the versions it supports
and kernel should avoid exposing such defines unless really needed.

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

Re: [Intel-gfx] [PATCH] drm/crc: Only open CRC on atomic drivers when the CRTC is active.

[Daniel Vetter]

On Thu, Jul 06, 2017 at 03:03:15PM +0200, Maarten Lankhorst wrote:
> Op 06-07-17 om 13:09 schreef Tomeu Vizoso:
> > Looks good to me:
> >
> > Reviewed-by: Tomeu Vizoso 
> >
> > I guess you have tested this with IGT? In any case, I think it would
> > be good to mention how a patch has been tested in the changelog. That
> > can be very useful to others if things go wrong at some point.
> Testcase: debugfs_test.read_all_entries
> 
> But I hit it by doing a recursive grep, which I guess is the same thing here. 
> :)
> 
> One further improvement I wanted to do was reject opening the CRC with -EIO
> when the crtc is not active, that way the above test will not hang.
> Does the below patch also look good to you?
> 
> 8<-
> Commit e8fa5671183c ("drm: crc: Wait for a frame before returning
> from open()") adds a wait for CRC frame, but with the CRTC off
> this will never be generated. For atomic drivers we know if a CRTC
> is active through crtc_state->active, so when inactive reject the
> open with -EIO.
> 
> Signed-off-by: Maarten Lankhorst 
> Fixes: e8fa5671183c ("drm: crc: Wait for a frame before returning from 
> open()")
> Testcase: debugfs_test.read_all_entries

At least for the semantics I think this makes sense. Opening the CRC file
when the crtc is off is undefined.

Reviewed-by: Daniel Vetter 

But pls get Tomeu's ack too.

Thanks, Daniel


> ---
> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c 
> b/drivers/gpu/drm/drm_debugfs_crc.c
> index d0ea4627a093..f9e26dda56d6 100644
> --- a/drivers/gpu/drm/drm_debugfs_crc.c
> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> @@ -154,6 +154,19 @@ static int crtc_crc_open(struct inode *inode, struct 
> file *filep)
>   size_t values_cnt;
>   int ret = 0;
>  
> + if (drm_drv_uses_atomic_modeset(crtc->dev)) {
> + ret = drm_modeset_lock_interruptible(>mutex, NULL);
> + if (ret)
> + return ret;
> +
> + if (!crtc->state->active)
> + ret = -EIO;
> + drm_modeset_unlock(>mutex);
> +
> + if (ret)
> + return ret;
> + }
> +
>   spin_lock_irq(>lock);
>   if (!crc->opened)
>   crc->opened = true;
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] Skylake / (EE) modeset(0): present flip failed loop

[Chris Wilson]

Quoting Marc MERLIN (2017-07-07 06:40:51)
> Is this the right place to send this?
> Can anyone help?
> 
> On Wed, Jul 05, 2017 at 11:33:01PM -0700, Marc MERLIN wrote:
> > Howdy,
> > 
> > I have a thinkpad P70 with debian testing and 4.11.6 kernel.
> > A recent-ish upgrade broke something and now I'm getting loads of spam
> > in my Xorg.log
> > 
> > [  5031.435] (WW) modeset(0): flip queue failed: Invalid argument
> > [  5031.435] (WW) modeset(0): Page flip failed: Invalid argument
> > [  5031.435] (EE) modeset(0): present flip failed
> > [  5031.519] (WW) modeset(0): flip queue failed: Invalid argument
> > [  5031.519] (WW) modeset(0): Page flip failed: Invalid argument
> > [  5031.519] (EE) modeset(0): present flip failed
> > (...)
> > 
> > system info:
> > ii  libdrm-intel1:amd642.4.74-1   
> > ii  xserver-xorg-core  2:1.19.2-1  
> > ii  xserver-xorg-video-intel   2:2.99.917+git20161206-1

If you were indeed using -intel then I would be more concerned.

For a page flip to fail, is to be expected. It can fail for a number of
reasons, most commonly either for a dp-mst disappearing or the
framebuffer to be incompatible with a pageflip.

-intel tries much harder (i.e. it tries at all) to handle the expected
failures than -modesetting.

But at the very least you need to dig into dmesg (with drm.debug=fe) to
find out why it failed. (One way is to run -intel with debugging enabled
so that it includes the kernel error messages along with the failure
message.)
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/1] drm/i915: Version the MOCS settings

[Chris Wilson]

Quoting Ben Widawsky (2017-07-07 00:27:01)
>  drivers/gpu/drm/i915/i915_drv.c |  3 +++
>  drivers/gpu/drm/i915/i915_drv.h |  2 ++
>  drivers/gpu/drm/i915/i915_pci.c | 13 +
>  include/uapi/drm/i915_drm.h |  8 
>  4 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9167a73f3c69..26c27b6ae814 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -401,6 +401,9 @@ static int i915_getparam(struct drm_device *dev, void 
> *data,
> if (!value)
> return -ENODEV;
> break;
> +   case I915_PARAM_MOCS_TABLE_VERSION:
> +   value = INTEL_INFO(dev_priv)->mocs_version;

If we use intel_mocs_get_table_version() we can put this magic number
in intel_mocs.c next to the tables, where we can keep its history and
hopefully be able to remember to update it.

> +/* What version of the MOCS table we have. For GEN9 GPUs, the PRM defined
> + * non-optimal settings for the MOCS table. As a result, we were required to 
> use a
> + * small subset, and later add new settings. This param allows userspace to
> + * determine which settings are there.
> + */
> +#define MOCS_TABLE_VERSION   1 /* Build time MOCS table version 
> */

How are you planing to share this? When we update we bump this number,
and then mesa copies it across and uses it after verifying it as 0,1 on
an old kernel.

I don't think you want to expose the updated constant here, but symbolic
names for each version? (What would be the point?)

Next question, why a version number and not just the number of entries
defined? Each index is defined by ABI once assigned, so the number of
entries still operates as a version number and allows easy checking.

if (advanced_cacheing_idx < kernel_max_mocs)
return advanced_cacheing_idx;
if (default_cacheing_idx < kernel_max_mocs)
return default_cacheing_idx;

return follow_pte_idx;

give or take the smarts to choose the preferred indices for any
particular scenario.

In the future, if we finally get user defined mocs, the table_size will
then give the start of the user modifiable indices (presming they want
to keep the predefined entries for compatibility?))
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 3/3] drm/i915: Consolidate max_cdclk_freq check in intel_crtc_compute_min_cdclk()

[ville . syrjala]

From: Ville Syrjälä 

Currently the .modeset_calc_cdclk() hooks check the final cdclk value
against the max allowed. That's not really sufficient since the low
level calc_cdclk() functions effectively clamp the minimum required
cdclk to the max supported by the platform. Hence if the minimum
required exceeds the platforms capabilities we'd keep going anyway
using the max cdclk frequency.

To fix that let's move the check earlier into
intel_crtc_compute_min_cdclk() and we'll check the minimum required
cdclk of the pipe against the maximum supported by the platform.

Cc: Paulo Zanoni 
Cc: Rodrigo Vivi 
Cc: Dhinakaran Pandiyan 
Cc: Maarten Lankhorst 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_cdclk.c   | 96 +---
 drivers/gpu/drm/i915/intel_display.c |  5 +-
 2 files changed, 48 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c 
b/drivers/gpu/drm/i915/intel_cdclk.c
index 82e5bc967cca..d7a77123a7e5 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1784,6 +1784,12 @@ int intel_crtc_compute_min_cdclk(const struct 
intel_crtc_state *crtc_state)
if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
min_cdclk = max(2 * 96000, min_cdclk);
 
+   if (min_cdclk > dev_priv->max_cdclk_freq) {
+   DRM_DEBUG_KMS("required cdclk (%d kHz) exceeds max (%d kHz)\n",
+ min_cdclk, dev_priv->max_cdclk_freq);
+   return -EINVAL;
+   }
+
return min_cdclk;
 }
 
@@ -1793,16 +1799,21 @@ static int intel_compute_min_cdclk(struct 
drm_atomic_state *state)
struct drm_i915_private *dev_priv = to_i915(state->dev);
struct intel_crtc *crtc;
struct intel_crtc_state *crtc_state;
-   int min_cdclk = 0, i;
+   int min_cdclk, i;
enum pipe pipe;
 
memcpy(intel_state->min_cdclk, dev_priv->min_cdclk,
   sizeof(intel_state->min_cdclk));
 
-   for_each_new_intel_crtc_in_state(intel_state, crtc, crtc_state, i)
-   intel_state->min_cdclk[i] =
-   intel_crtc_compute_min_cdclk(crtc_state);
+   for_each_new_intel_crtc_in_state(intel_state, crtc, crtc_state, i) {
+   min_cdclk = intel_crtc_compute_min_cdclk(crtc_state);
+   if (min_cdclk < 0)
+   return min_cdclk;
+
+   intel_state->min_cdclk[i] = min_cdclk;
+   }
 
+   min_cdclk = 0;
for_each_pipe(dev_priv, pipe)
min_cdclk = max(intel_state->min_cdclk[pipe], min_cdclk);
 
@@ -1812,18 +1823,14 @@ static int intel_compute_min_cdclk(struct 
drm_atomic_state *state)
 static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
struct drm_i915_private *dev_priv = to_i915(state->dev);
-   int min_cdclk = intel_compute_min_cdclk(state);
-   struct intel_atomic_state *intel_state =
-   to_intel_atomic_state(state);
-   int cdclk;
+   struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+   int min_cdclk, cdclk;
 
-   cdclk = vlv_calc_cdclk(dev_priv, min_cdclk);
+   min_cdclk = intel_compute_min_cdclk(state);
+   if (min_cdclk < 0)
+   return min_cdclk;
 
-   if (cdclk > dev_priv->max_cdclk_freq) {
-   DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
- cdclk, dev_priv->max_cdclk_freq);
-   return -EINVAL;
-   }
+   cdclk = vlv_calc_cdclk(dev_priv, min_cdclk);
 
intel_state->cdclk.logical.cdclk = cdclk;
 
@@ -1841,10 +1848,12 @@ static int vlv_modeset_calc_cdclk(struct 
drm_atomic_state *state)
 
 static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
-   struct drm_i915_private *dev_priv = to_i915(state->dev);
struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
-   int min_cdclk = intel_compute_min_cdclk(state);
-   int cdclk;
+   int min_cdclk, cdclk;
+
+   min_cdclk = intel_compute_min_cdclk(state);
+   if (min_cdclk < 0)
+   return min_cdclk;
 
/*
 * FIXME should also account for plane ratio
@@ -1852,12 +1861,6 @@ static int bdw_modeset_calc_cdclk(struct 
drm_atomic_state *state)
 */
cdclk = bdw_calc_cdclk(min_cdclk);
 
-   if (cdclk > dev_priv->max_cdclk_freq) {
-   DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
- cdclk, dev_priv->max_cdclk_freq);
-   return -EINVAL;
-   }
-
intel_state->cdclk.logical.cdclk = cdclk;
 
if (!intel_state->active_crtcs) {
@@ -1874,10 +1877,13 @@ static int bdw_modeset_calc_cdclk(struct 
drm_atomic_state *state)
 
 static int 

[Intel-gfx] [PATCH 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock"

[ville . syrjala]

From: Ville Syrjälä 

Make the min_pixclk thing less confusing by changing it to track
the minimum acceptable cdclk frequency instead. This means moving
the application of the guardbands to a slightly higher level from
the low level platform specific calc_cdclk() functions.

The immediate benefit is elimination of the confusing 2x factors
on GLK/CNL+ in the audio workarounds (which stems from the fact
that the pipes produce two pixels per clock).

Cc: Paulo Zanoni 
Cc: Rodrigo Vivi 
Cc: Dhinakaran Pandiyan 
Cc: Maarten Lankhorst 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_drv.h  |  12 ++-
 drivers/gpu/drm/i915/intel_cdclk.c   | 179 +--
 drivers/gpu/drm/i915/intel_display.c |  21 ++--
 drivers/gpu/drm/i915/intel_drv.h |   4 +-
 4 files changed, 107 insertions(+), 109 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 81cd21ecfa7d..c80176424d84 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -563,6 +563,15 @@ struct i915_hotplug {
 (__i)++) \
for_each_if (plane_state)
 
+#define for_each_new_intel_crtc_in_state(__state, crtc, new_crtc_state, __i) \
+   for ((__i) = 0; \
+(__i) < (__state)->base.dev->mode_config.num_crtc && \
+((crtc) = to_intel_crtc((__state)->base.crtcs[__i].ptr), \
+ (new_crtc_state) = 
to_intel_crtc_state((__state)->base.crtcs[__i].new_state), 1); \
+(__i)++) \
+   for_each_if (crtc)
+
+
 struct drm_i915_private;
 struct i915_mm_struct;
 struct i915_mmu_object;
@@ -2268,7 +2277,8 @@ struct drm_i915_private {
struct mutex dpll_lock;
 
unsigned int active_crtcs;
-   unsigned int min_pixclk[I915_MAX_PIPES];
+   /* minimum acceptable cdclk for each pipe */
+   int min_cdclk[I915_MAX_PIPES];
 
int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
 
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c 
b/drivers/gpu/drm/i915/intel_cdclk.c
index 9e0deebae279..82e5bc967cca 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -417,24 +417,21 @@ static void hsw_get_cdclk(struct drm_i915_private 
*dev_priv,
cdclk_state->cdclk = 54;
 }
 
-static int vlv_calc_cdclk(struct drm_i915_private *dev_priv,
- int max_pixclk)
+static int vlv_calc_cdclk(struct drm_i915_private *dev_priv, int min_cdclk)
 {
int freq_320 = (dev_priv->hpll_freq <<  1) % 32 != 0 ?
33 : 32;
-   int limit = IS_CHERRYVIEW(dev_priv) ? 95 : 90;
 
/*
 * We seem to get an unstable or solid color picture at 200MHz.
 * Not sure what's wrong. For now use 200MHz only when all pipes
 * are off.
 */
-   if (!IS_CHERRYVIEW(dev_priv) &&
-   max_pixclk > freq_320*limit/100)
+   if (IS_VALLEYVIEW(dev_priv) && min_cdclk > freq_320)
return 40;
-   else if (max_pixclk > 27*limit/100)
+   else if (min_cdclk > 27)
return freq_320;
-   else if (max_pixclk > 0)
+   else if (min_cdclk > 0)
return 27;
else
return 20;
@@ -612,13 +609,13 @@ static void chv_set_cdclk(struct drm_i915_private 
*dev_priv,
intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
 }
 
-static int bdw_calc_cdclk(int max_pixclk)
+static int bdw_calc_cdclk(int min_cdclk)
 {
-   if (max_pixclk > 54)
+   if (min_cdclk > 54)
return 675000;
-   else if (max_pixclk > 45)
+   else if (min_cdclk > 45)
return 54;
-   else if (max_pixclk > 337500)
+   else if (min_cdclk > 337500)
return 45;
else
return 337500;
@@ -724,23 +721,23 @@ static void bdw_set_cdclk(struct drm_i915_private 
*dev_priv,
 cdclk, dev_priv->cdclk.hw.cdclk);
 }
 
-static int skl_calc_cdclk(int max_pixclk, int vco)
+static int skl_calc_cdclk(int min_cdclk, int vco)
 {
if (vco == 864) {
-   if (max_pixclk > 54)
+   if (min_cdclk > 54)
return 617143;
-   else if (max_pixclk > 432000)
+   else if (min_cdclk > 432000)
return 54;
-   else if (max_pixclk > 308571)
+   else if (min_cdclk > 308571)
return 432000;
else
return 308571;
} else {
-   if (max_pixclk > 54)
+   if (min_cdclk > 54)
return 675000;
-   else if (max_pixclk > 45)
+   else if (min_cdclk > 45)
   

[Intel-gfx] [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits

[ville . syrjala]

From: Ville Syrjälä 

Follow the GLK path when computing cdclk and related limits. CNL
pipes also produce two pixels per clock, so that's what we should
use.

For the HBR2 vs. audio issue the limit should more correctly be 336
MHz, but the GLK limit of 316.8 MHz works just as well and results
in picking at least 336 MHz. Also toss in some related w/a numbers.

Cc: Paulo Zanoni 
Cc: Rodrigo Vivi 
Cc: Dhinakaran Pandiyan 
Cc: Maarten Lankhorst 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_cdclk.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c 
b/drivers/gpu/drm/i915/intel_cdclk.c
index 1241e5891b29..9e0deebae279 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct 
intel_crtc_state *crtc_state,
crtc_state->has_audio &&
crtc_state->port_clock >= 54 &&
crtc_state->lane_count == 4) {
-   if (IS_CANNONLAKE(dev_priv))
-   pixel_rate = max(316800, pixel_rate);
-   else if (IS_GEMINILAKE(dev_priv))
+   if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
+   /* Display WA #1145: glk,cnl */
pixel_rate = max(2 * 316800, pixel_rate);
-   else
+   } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
+   /* Display WA #1144: skl,bxt */
pixel_rate = max(432000, pixel_rate);
+   }
}
 
/* According to BSpec, "The CD clock frequency must be at least twice
@@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct 
intel_crtc_state *crtc_state,
 * two pixels per clock.
 */
if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
-   if (IS_GEMINILAKE(dev_priv))
+   if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
pixel_rate = max(2 * 2 * 96000, pixel_rate);
else
pixel_rate = max(2 * 96000, pixel_rate);
@@ -1999,7 +2000,9 @@ static int intel_compute_max_dotclk(struct 
drm_i915_private *dev_priv)
 {
int max_cdclk_freq = dev_priv->max_cdclk_freq;
 
-   if (IS_GEMINILAKE(dev_priv))
+   if (IS_CANNONLAKE(dev_priv))
+   return 2 * max_cdclk_freq;
+   else if (IS_GEMINILAKE(dev_priv))
/*
 * FIXME: Limiting to 99% as a temporary workaround. See
 * glk_calc_cdclk() for details.
-- 
2.13.0

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

Re: [Intel-gfx] [PATCH 13/15] drm/i915: Allow execbuffer to use the first object as the batch

[Daniel Vetter]

On Fri, Mar 17, 2017 at 12:15 PM, Joonas Lahtinen
 wrote:
> On to, 2017-03-16 at 13:20 +, Chris Wilson wrote:
>> Currently, the last object in the execlist is the always the batch.
>> However, when building the batch buffer we often know the batch object
>> first and if we can use the first slot in the execlist we can emit
>> relocation instructions relative to it immediately and avoid a separate
>> pass to adjust the relocations to point to the last execlist slot.
>>
>> Signed-off-by: Chris Wilson 
>
> Reviewed-by: Joonas Lahtinen 

This patch was reviewed/pushed full month before the mesa patch was
fully reviewed and ready for merging. That's not how uapi is done.

I've fixed this up now by at least reviewing the mesa patch, but for
next time around: If you review uapi, and you don't make sure the
userspace side is in good shape too, then you've not reviewed the
patch properly.

Same goes for reviewing and not making sure there's tests, but that's
another rant.

Ken, pls make sure we don't end up with another case like resource
streamer (or tell me I should revert this).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Fix the kernel panic when using aliasing ppgtt (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Fix the kernel panic when using aliasing ppgtt (rev2)
URL   : https://patchwork.freedesktop.org/series/26977/
State : success

== Summary ==

Series 26977v2 drm/i915: Fix the kernel panic when using aliasing ppgtt
https://patchwork.freedesktop.org/api/1.0/series/26977/revisions/2/mbox/

Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass   -> DMESG-WARN (fi-kbl-r) fdo#100125
Test kms_flip:
Subgroup basic-flip-vs-modeset:
skip   -> PASS   (fi-skl-x1585l)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
dmesg-warn -> PASS   (fi-pnv-d510) fdo#101597 +1

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:439s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:426s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:353s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:525s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:507s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:485s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:487s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:594s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:435s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:412s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:418s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:495s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:482s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:460s
fi-kbl-7560u total:279  pass:268  dwarn:1   dfail:0   fail:0   skip:10  
time:567s
fi-kbl-r total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  
time:582s
fi-pnv-d510  total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  
time:563s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:465s
fi-skl-6700hqtotal:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  
time:588s
fi-skl-6700k total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  
time:466s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:480s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:439s
fi-skl-x1585ltotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:490s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:539s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:401s

11125fb7b775223081da697898a92119cb017538 drm-tip: 2017y-07m-06d-23h-19m-02s UTC 
integration manifest
6ba5357 drm/i915: Fix the kernel panic when using aliasing ppgtt

== Logs ==

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

Re: [Intel-gfx] [PATCH v2] drm/i915: Fix the kernel panic when using aliasing ppgtt

[Chris Wilson]

Quoting Chuanxiao Dong (2017-07-07 10:50:59)
> The ppgtt should be get directly from i915_address_space *vm instead of
> vma->vm.
> 
> v2:
> - add one more fix for bxt. (Chris)
> 
> Fixes: 4a234c5fae16 ("drm/i915: pass the vma to insert_entries")
> Bugzilla:https://bugs.freedesktop.org/show_bug.cgi?id=101713
> Signed-off-by: Chuanxiao Dong 
> Reviewed-by: Matthew Auld  v1
> Cc: Matthew Auld 
> Cc: Chris Wilson 
> Cc: Zhenyu Wang 

Thanks for finding and quickly providing the fix, pushed.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2] drm/i915: Fix the kernel panic when using aliasing ppgtt

[Chuanxiao Dong]

The ppgtt should be get directly from i915_address_space *vm instead of
vma->vm.

v2:
- add one more fix for bxt. (Chris)

Fixes: 4a234c5fae16 ("drm/i915: pass the vma to insert_entries")
Bugzilla:https://bugs.freedesktop.org/show_bug.cgi?id=101713
Signed-off-by: Chuanxiao Dong 
Reviewed-by: Matthew Auld  v1
Cc: Matthew Auld 
Cc: Chris Wilson 
Cc: Zhenyu Wang 
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index de67084..10aa776 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -910,7 +910,7 @@ static void gen8_ppgtt_insert_3lvl(struct 
i915_address_space *vm,
   enum i915_cache_level cache_level,
   u32 unused)
 {
-   struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vma->vm);
+   struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
struct sgt_dma iter = {
.sg = vma->pages->sgl,
.dma = sg_dma_address(iter.sg),
@@ -2242,7 +2242,7 @@ static void bxt_vtd_ggtt_insert_entries__BKL(struct 
i915_address_space *vm,
 enum i915_cache_level level,
 u32 unused)
 {
-   struct insert_entries arg = { vma->vm, vma, level };
+   struct insert_entries arg = { vm, vma, level };
 
stop_machine(bxt_vtd_ggtt_insert_entries__cb, , NULL);
 }
-- 
2.7.4

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

Re: [Intel-gfx] [PATCH] drm/i915: Fix the kernel panic when using aliasing ppgtt

[Dong, Chuanxiao]

> -Original Message-
> From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> Sent: Friday, July 7, 2017 5:38 PM
> To: Dong, Chuanxiao ; intel-
> g...@lists.freedesktop.org
> Cc: intel-gvt-...@lists.freedesktop.org; Dong, Chuanxiao
> ; Auld, Matthew ;
> Zhenyu Wang 
> Subject: Re: [PATCH] drm/i915: Fix the kernel panic when using aliasing ppgtt
> 
> Quoting Chuanxiao Dong (2017-07-07 07:00:09)
> > The ppgtt should be get directly from i915_address_space *vm instead
> > of
> > vma->vm as in alias ppgtt case the vma->vm is not same with vm.
> 
> And for consistency, also
> 
> @@ -2242,7 +2242,7 @@ static void
> bxt_vtd_ggtt_insert_entries__BKL(struct i915_address_space *vm,
>  enum i915_cache_level level,
>  u32 unused)  {
> -   struct insert_entries arg = { vma->vm, vma, level };
> +   struct insert_entries arg = { vm, vma, level };
> 
> stop_machine(bxt_vtd_ggtt_insert_entries__cb, , NULL);  }

Good catch! Will send out v2 to include this.

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

Re: [Intel-gfx] [PATCH] drm/i915: Fix the kernel panic when using aliasing ppgtt

[Chris Wilson]

Quoting Chuanxiao Dong (2017-07-07 07:00:09)
> The ppgtt should be get directly from i915_address_space *vm instead of
> vma->vm as in alias ppgtt case the vma->vm is not same with vm.

And for consistency, also

@@ -2242,7 +2242,7 @@ static void bxt_vtd_ggtt_insert_entries__BKL(struct 
i915_address_space *vm,
 enum i915_cache_level level,
 u32 unused)
 {
-   struct insert_entries arg = { vma->vm, vma, level };
+   struct insert_entries arg = { vm, vma, level };
 
stop_machine(bxt_vtd_ggtt_insert_entries__cb, , NULL);
 }
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/cnl: Don't trust VBT's alternate pin for port D for now.

[Patchwork]

== Series Details ==

Series: drm/i915/cnl: Don't trust VBT's alternate pin for port D for now.
URL   : https://patchwork.freedesktop.org/series/26956/
State : success

== Summary ==

Series 26956v1 drm/i915/cnl: Don't trust VBT's alternate pin for port D for now.
https://patchwork.freedesktop.org/api/1.0/series/26956/revisions/1/mbox/

Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass   -> DMESG-WARN (fi-kbl-r) fdo#100125

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:452s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:429s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:357s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:517s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:505s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:497s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:480s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:595s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:433s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:409s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:422s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:499s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:481s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:465s
fi-kbl-7560u total:279  pass:268  dwarn:1   dfail:0   fail:0   skip:10  
time:569s
fi-kbl-r total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  
time:580s
fi-pnv-d510  total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  
time:564s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:460s
fi-skl-6700hqtotal:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  
time:584s
fi-skl-6700k total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  
time:468s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:477s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:437s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:542s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:400s

11125fb7b775223081da697898a92119cb017538 drm-tip: 2017y-07m-06d-23h-19m-02s UTC 
integration manifest
6514db7 drm/i915/cnl: Don't trust VBT's alternate pin for port D for now.

== Logs ==

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

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v7,1/2] drm/i915: Enable guest i915 48bit full ppgtt functionality

[Patchwork]

== Series Details ==

Series: series starting with [v7,1/2] drm/i915: Enable guest i915 48bit full 
ppgtt functionality
URL   : https://patchwork.freedesktop.org/series/26980/
State : success

== Summary ==

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

Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
pass   -> FAIL   (fi-snb-2600) fdo#17
Test gem_exec_suspend:
Subgroup basic-s4-devices:
dmesg-warn -> PASS   (fi-kbl-7560u) fdo#100125 +1
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
dmesg-warn -> PASS   (fi-pnv-d510) fdo#101597

fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:447s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:432s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:358s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:523s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:505s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:486s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:485s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:601s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:431s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:414s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:423s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:501s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:474s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:464s
fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:579s
fi-kbl-r total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  
time:584s
fi-pnv-d510  total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  
time:564s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:461s
fi-skl-6700hqtotal:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  
time:586s
fi-skl-6700k total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  
time:466s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:476s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:443s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:539s
fi-snb-2600  total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  
time:406s

11125fb7b775223081da697898a92119cb017538 drm-tip: 2017y-07m-06d-23h-19m-02s UTC 
integration manifest
4ef7bf7 drm/i915/gvt: Fix guest i915 48bit full ppgtt blocking issue
d770cdf drm/i915: Enable guest i915 48bit full ppgtt functionality

== Logs ==

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

Re: [Intel-gfx] [PATCH] drm/i915: Fix the kernel panic when using aliasing ppgtt

[Matthew Auld]

On 7 July 2017 at 07:00, Chuanxiao Dong  wrote:
> The ppgtt should be get directly from i915_address_space *vm instead of
> vma->vm as in alias ppgtt case the vma->vm is not same with vm.
>
> Fixes: 4a234c5fae16 ("drm/i915: pass the vma to insert_entries")
> Bugzilla:https://bugs.freedesktop.org/show_bug.cgi?id=101713
> Signed-off-by: Chuanxiao Dong 
> Cc: Matthew Auld 
> Cc: Chris Wilson 
> Cc: Zhenyu Wang 
Reviewed-by: Matthew Auld 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Fix the kernel panic when using aliasing ppgtt

[Patchwork]

== Series Details ==

Series: drm/i915: Fix the kernel panic when using aliasing ppgtt
URL   : https://patchwork.freedesktop.org/series/26977/
State : warning

== Summary ==

Series 26977v1 drm/i915: Fix the kernel panic when using aliasing ppgtt
https://patchwork.freedesktop.org/api/1.0/series/26977/revisions/1/mbox/

Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass   -> DMESG-WARN (fi-kbl-r) fdo#100125
Test gem_ringfill:
Subgroup basic-default:
pass   -> SKIP   (fi-bsw-n3050)
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-legacy:
pass   -> FAIL   (fi-snb-2600) fdo#100215
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
dmesg-warn -> PASS   (fi-pnv-d510) fdo#101597 +1
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> PASS   (fi-byt-j1900) fdo#101705
Subgroup suspend-read-crc-pipe-c:
pass   -> FAIL   (fi-skl-6700k) fdo#100367

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705
fdo#100367 https://bugs.freedesktop.org/show_bug.cgi?id=100367

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:440s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:428s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:359s
fi-bsw-n3050 total:279  pass:242  dwarn:0   dfail:0   fail:0   skip:37  
time:522s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:506s
fi-byt-j1900 total:279  pass:255  dwarn:0   dfail:0   fail:0   skip:24  
time:485s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:484s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:595s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:439s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:408s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:415s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:500s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:478s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:460s
fi-kbl-7560u total:279  pass:268  dwarn:1   dfail:0   fail:0   skip:10  
time:571s
fi-kbl-r total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  
time:573s
fi-pnv-d510  total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  
time:559s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:460s
fi-skl-6700hqtotal:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  
time:587s
fi-skl-6700k total:279  pass:256  dwarn:4   dfail:0   fail:1   skip:18  
time:474s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:471s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:435s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:542s
fi-snb-2600  total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  
time:406s

11125fb7b775223081da697898a92119cb017538 drm-tip: 2017y-07m-06d-23h-19m-02s UTC 
integration manifest
a4dd1e6 drm/i915: Fix the kernel panic when using aliasing ppgtt

== Logs ==

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

[Intel-gfx] ✓ Fi.CI.BAT: success for YCBCR 4:2:0 handling in DRM layer

[Patchwork]

== Series Details ==

Series: YCBCR 4:2:0 handling in DRM layer
URL   : https://patchwork.freedesktop.org/series/26972/
State : success

== Summary ==

Series 26972v1 YCBCR 4:2:0 handling in DRM layer
https://patchwork.freedesktop.org/api/1.0/series/26972/revisions/1/mbox/

Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
pass   -> FAIL   (fi-snb-2600) fdo#17
Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass   -> DMESG-WARN (fi-kbl-r) fdo#100125

fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:444s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:429s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:356s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:529s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:506s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:489s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:486s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:592s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:439s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:413s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:420s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:488s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:494s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:463s
fi-kbl-7560u total:279  pass:268  dwarn:1   dfail:0   fail:0   skip:10  
time:570s
fi-kbl-r total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  
time:581s
fi-pnv-d510  total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  
time:565s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:457s
fi-skl-6700hqtotal:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  
time:583s
fi-skl-6700k total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  
time:470s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:478s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:432s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:539s
fi-snb-2600  total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  
time:407s

11125fb7b775223081da697898a92119cb017538 drm-tip: 2017y-07m-06d-23h-19m-02s UTC 
integration manifest
99654cb drm/i915: add config function for YCBCR420 outputs
ae15c39 drm: add helper functions for YCBCR420 handling
ff94e1c drm: set output colorspace in AVI infoframe
401387c drm/edid: parse ycbcr 420 deep color information
ae94beb drm: add helper to validate YCBCR420 modes
d6ab3d0 drm/edid: parse YCBCR420 videomodes from EDID
8a41dee drm/edid: cleanup patch for CEA extended-tag macro
586ddfe drm/edid: parse sink information before CEA blocks
2352b25 drm/edid: complete CEA modedb(VIC 1-107)
8f9fbfc drm: handle HDMI 2.0 VICs in AVI info-frames

== Logs ==

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

Re: [Intel-gfx] [PATCH v7 1/2] drm/i915: Enable guest i915 48bit full ppgtt functionality

[Zhenyu Wang]

On 2017.07.07 15:29:38 +0800, Zhenyu Wang wrote:
> From: Tina Zhang 
> 
> Enable the guest i915 48bit full ppgtt functionality when host can provide
> the capability. vgt_caps is introduced to guest i915 driver to get the vgpu
> capabilities from the device model. VGT_CAPS_FULL_PPGTT_48BIT is one of the
> capabilities type which let guest i915 dirver know that the guest 48bit
> i915 full ppgtt is supported by device model.
> 
> Notice that the minor version of pvinfo isn't bumped because of this
> vgt_caps introduction, due to older guest would be broken by simply
> increasing the pvinfo version. Although the pvinfo minor version doesn't
> increase, the compatibility won't be blocked. The compatibility is ensured
> by checking the value of caps field in pvinfo. Zero means no full ppgtt
> support and BIT(2) means this feature is provided.
>

Note: as agreed with Joonas if i915 change is reviewed ok, this series
will be merged through gvt tree for dependence.

-- 
Open Source Technology Center, Intel ltd.

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


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

[Intel-gfx] [PATCH v7 2/2] drm/i915/gvt: Fix guest i915 48bit full ppgtt blocking issue

[Zhenyu Wang]

From: Tina Zhang 

Guest i915 48bit full ppgtt functionality was blocking by an issue, which
would lead to gpu hardware hang. Guest i915 driver may update the ppgtt
table just before this workload is going to be submitted to the hardware
by device model. This case wasn't handled well by device model before, due
to the small time window between removing old ppgtt entry and adding the
new one. Errors occur when the workload is executed by hardware during
that small time window. This patch is to remove this time window by adding
the new ppgtt entry first and then remove the old one.

Changes in v2:
- Move VGT_CAPS_FULL_PPGTT introduction to patch 2/4. (Joonas)

Changes since v2:
- Divide the whole patch set into two separate patch series, with one
  patch in i915 side to check guest i915 full ppgtt capability and enable
  it when this capability is supported by the device model, and the other
  one in gvt side which fixs the blocking issue and enables the device
  model to provide the capability to guest. And this patch focuses on gvt
  side. (Joonas)
- Change the title from "reorder the shadow ppgtt update process by adding
  entry first" to "Fix guest i915 full ppgtt blocking issue". (Tina)

Changes since v3:
- Rebase to the latest branch.

Changes since v4:
- Tested by Tina Zhang.

Changes since v5:
- Rebase to the latest branch.

Signed-off-by: Tina Zhang 
Signed-off-by: Zhenyu Wang 
---
 drivers/gpu/drm/i915/gvt/gtt.c  | 45 +
 drivers/gpu/drm/i915/gvt/vgpu.c |  1 +
 2 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index 6166e34d892b..27bda426d42e 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -979,29 +979,26 @@ static int ppgtt_populate_shadow_page(struct 
intel_vgpu_ppgtt_spt *spt)
 }
 
 static int ppgtt_handle_guest_entry_removal(struct intel_vgpu_guest_page *gpt,
-   unsigned long index)
+   struct intel_gvt_gtt_entry *se, unsigned long index)
 {
struct intel_vgpu_ppgtt_spt *spt = guest_page_to_ppgtt_spt(gpt);
struct intel_vgpu_shadow_page *sp = >shadow_page;
struct intel_vgpu *vgpu = spt->vgpu;
struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops;
-   struct intel_gvt_gtt_entry e;
int ret;
 
-   ppgtt_get_shadow_entry(spt, , index);
-
-   trace_gpt_change(spt->vgpu->id, "remove", spt, sp->type, e.val64,
+   trace_gpt_change(spt->vgpu->id, "remove", spt, sp->type, se->val64,
 index);
 
-   if (!ops->test_present())
+   if (!ops->test_present(se))
return 0;
 
-   if (ops->get_pfn() == vgpu->gtt.scratch_pt[sp->type].page_mfn)
+   if (ops->get_pfn(se) == vgpu->gtt.scratch_pt[sp->type].page_mfn)
return 0;
 
-   if (gtt_type_is_pt(get_next_pt_type(e.type))) {
+   if (gtt_type_is_pt(get_next_pt_type(se->type))) {
struct intel_vgpu_ppgtt_spt *s =
-   ppgtt_find_shadow_page(vgpu, ops->get_pfn());
+   ppgtt_find_shadow_page(vgpu, ops->get_pfn(se));
if (!s) {
gvt_vgpu_err("fail to find guest page\n");
ret = -ENXIO;
@@ -1011,12 +1008,10 @@ static int ppgtt_handle_guest_entry_removal(struct 
intel_vgpu_guest_page *gpt,
if (ret)
goto fail;
}
-   ops->set_pfn(, vgpu->gtt.scratch_pt[sp->type].page_mfn);
-   ppgtt_set_shadow_entry(spt, , index);
return 0;
 fail:
gvt_vgpu_err("fail: shadow page %p guest entry 0x%llx type %d\n",
-   spt, e.val64, e.type);
+   spt, se->val64, se->type);
return ret;
 }
 
@@ -1236,22 +1231,37 @@ static int ppgtt_handle_guest_write_page_table(
 {
struct intel_vgpu_ppgtt_spt *spt = guest_page_to_ppgtt_spt(gpt);
struct intel_vgpu *vgpu = spt->vgpu;
+   int type = spt->shadow_page.type;
struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops;
+   struct intel_gvt_gtt_entry se;
 
int ret;
int new_present;
 
new_present = ops->test_present(we);
 
-   ret = ppgtt_handle_guest_entry_removal(gpt, index);
-   if (ret)
-   goto fail;
+   /*
+* Adding the new entry first and then removing the old one, that can
+* guarantee the ppgtt table is validated during the window between
+* adding and removal.
+*/
+   ppgtt_get_shadow_entry(spt, , index);
 
if (new_present) {
ret = ppgtt_handle_guest_entry_add(gpt, we, index);
if (ret)
goto fail;
}
+
+   ret = ppgtt_handle_guest_entry_removal(gpt, , index);
+   if (ret)
+   goto fail;
+
+   if (!new_present) {
+   

[Intel-gfx] [PATCH v7 1/2] drm/i915: Enable guest i915 48bit full ppgtt functionality

[Zhenyu Wang]

From: Tina Zhang 

Enable the guest i915 48bit full ppgtt functionality when host can provide
the capability. vgt_caps is introduced to guest i915 driver to get the vgpu
capabilities from the device model. VGT_CAPS_FULL_PPGTT_48BIT is one of the
capabilities type which let guest i915 dirver know that the guest 48bit
i915 full ppgtt is supported by device model.

Notice that the minor version of pvinfo isn't bumped because of this
vgt_caps introduction, due to older guest would be broken by simply
increasing the pvinfo version. Although the pvinfo minor version doesn't
increase, the compatibility won't be blocked. The compatibility is ensured
by checking the value of caps field in pvinfo. Zero means no full ppgtt
support and BIT(2) means this feature is provided.

Changes since v1:
- Use u32 instead of uint32_t (Joonas)
- Move VGT_CAPS_FULL_PPGTT introduction to this patch and use #define
  instead of enum (Joonas)
- Rewrite the vgpu full ppgtt capability checking logic. (Joonas)
- Some coding style refine. (Joonas)

Changes since v2:
- Divide the whole patch set into two separate patch series, with one
  patch in i915 side to check guest i915 full ppgtt capability and enable
  it when this capability is supported by the device model, and the other
  one in gvt side which fixs the blocking issue and enables the device
  model to provide the capability to guest. And this patch focuses on guest
  i915 side. (Joonas)
- Change the title from "introduce vgt_caps to pvinfo" to
  "Enable guest i915 full ppgtt functionality". (Tina)

Change since v3:
- Add some comments about pvinfo caps and version. (Joonas)

Change since v4:
- Tested by Tina Zhang.

Change since v5:
- Add limitation about supporting 32bit full ppgtt.

Change since v6:
- Cleanup and fix previous 32bit ppgtt setting check and favor to
  enable 48bit ppgtt if host gvt-g provides support. (Zhenyu)

Reviewed-by: Joonas Lahtinen  in v2
Signed-off-by: Tina Zhang 
Signed-off-by: Zhenyu Wang 
---
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_gem_gtt.c | 13 +
 drivers/gpu/drm/i915/i915_pvinfo.h  |  8 +++-
 drivers/gpu/drm/i915/i915_vgpu.c|  7 +++
 drivers/gpu/drm/i915/i915_vgpu.h|  3 +++
 5 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5e70f5711fc8..2903d7e9b8af 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1878,6 +1878,7 @@ struct i915_workarounds {
 
 struct i915_virtual_gpu {
bool active;
+   u32 caps;
 };
 
 /* used in computing the new watermarks state */
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index de67084d5fcf..07167dc32fdd 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -141,14 +141,19 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private 
*dev_priv,
 
has_aliasing_ppgtt = dev_priv->info.has_aliasing_ppgtt;
has_full_ppgtt = dev_priv->info.has_full_ppgtt;
-   has_full_48bit_ppgtt = dev_priv->info.has_full_48bit_ppgtt;
 
if (intel_vgpu_active(dev_priv)) {
-   /* emulation is too hard */
-   has_full_ppgtt = false;
-   has_full_48bit_ppgtt = false;
+   has_full_ppgtt = intel_vgpu_has_full_ppgtt(dev_priv);
+   /* GVT-g has no support for 32bit ppgtt */
+   if (enable_ppgtt == 2 && has_full_ppgtt) {
+   DRM_DEBUG_DRIVER("Force 48bit ppgtt for vGPU\n");
+   enable_ppgtt = 3;
+   }
}
 
+   has_full_48bit_ppgtt = has_full_ppgtt &&
+   dev_priv->info.has_full_48bit_ppgtt;
+
if (!has_aliasing_ppgtt)
return 0;
 
diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h 
b/drivers/gpu/drm/i915/i915_pvinfo.h
index 2cfe96d3e5d1..7e958d7f995f 100644
--- a/drivers/gpu/drm/i915/i915_pvinfo.h
+++ b/drivers/gpu/drm/i915/i915_pvinfo.h
@@ -49,12 +49,18 @@ enum vgt_g2v_type {
VGT_G2V_MAX,
 };
 
+/*
+ * VGT capabilities type
+ */
+#define VGT_CAPS_FULL_PPGTT_48BIT  BIT(2)
+
 struct vgt_if {
u64 magic;  /* VGT_MAGIC */
u16 version_major;
u16 version_minor;
u32 vgt_id; /* ID of vGT instance */
-   u32 rsv1[12];   /* pad to offset 0x40 */
+   u32 vgt_caps;   /* VGT capabilities */
+   u32 rsv1[11];   /* pad to offset 0x40 */
/*
 *  Data structure to describe the balooning info of resources.
 *  Each VM can only have one portion of continuous area for now.
diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index cf7a958e4d3c..e85e27c0d427 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -75,10 +75,17 @@ 

[Intel-gfx] [PATCH] drm/i915: Fix the kernel panic when using aliasing ppgtt

[Chuanxiao Dong]

The ppgtt should be get directly from i915_address_space *vm instead of
vma->vm as in alias ppgtt case the vma->vm is not same with vm.

Fixes: 4a234c5fae16 ("drm/i915: pass the vma to insert_entries")
Bugzilla:https://bugs.freedesktop.org/show_bug.cgi?id=101713
Signed-off-by: Chuanxiao Dong 
Cc: Matthew Auld 
Cc: Chris Wilson 
Cc: Zhenyu Wang 
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index de67084..867dcdc 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -910,7 +910,7 @@ static void gen8_ppgtt_insert_3lvl(struct 
i915_address_space *vm,
   enum i915_cache_level cache_level,
   u32 unused)
 {
-   struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vma->vm);
+   struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
struct sgt_dma iter = {
.sg = vma->pages->sgl,
.dma = sg_dma_address(iter.sg),
-- 
2.7.4

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