Re: [Intel-gfx] [PATCH 13/15] drm/i915: Allow execbuffer to use the first object as the batch
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
On 7 July 2017 at 18:08, Lionel Landwerlinwrote: > 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
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
== 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
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
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 NavareCc: 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
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 NavareCc: 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
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 Bridewrote: > > 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
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
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
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
On 17-07-07 09:23:26, Jason Ekstrand wrote: On Fri, Jul 7, 2017 at 3:34 AM, Chris Wilsonwrote: 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
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
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"
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
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
== 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
== 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.
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
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
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
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
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 WilsonCc: 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
From: Matthew AuldThe 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
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.
patch merged to dinq. On Thu, Jul 6, 2017 at 1:52 PM, Rodrigo Viviwrote: > 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
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.
patch merged to dinq. thanks for reviewing. On Thu, Jul 6, 2017 at 7:51 PM, Ben Widawskywrote: > 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.
patch merged to dinq. thanks for reviewing. On Fri, Jul 7, 2017 at 5:03 AM, David Weinehallwrote: > 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.
patch merged to dinq. thanks for reviewing. On Thu, Jul 6, 2017 at 2:52 PM, Clint Taylorwrote: > > > 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
On Thu, Jul 6, 2017 at 4:27 PM, Ben Widawskywrote: > 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
On Fri, Jul 7, 2017 at 3:34 AM, Chris Wilsonwrote: > 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()
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)
== 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
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()
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.
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.
On Fri, Jun 30, 2017 at 01:11:52PM -0700, Rodrigo Vivi wrote: > On Fri, Jun 30, 2017 at 12:12 PM, Jim Bridewrote: > > 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
On Thu, Jul 6, 2017 at 9:00 AM, Daniel Vetterwrote: > 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()
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.
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()
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
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
== 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
On 7 July 2017 at 11:34, Chris Wilsonwrote: > 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.
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
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
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()
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"
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
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
On Fri, Mar 17, 2017 at 12:15 PM, Joonas Lahtinenwrote: > 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)
== 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
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
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 DongReviewed-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
> -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
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.
== 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
== 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
On 7 July 2017 at 07:00, Chuanxiao Dongwrote: > 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
== 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
== 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
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
From: Tina ZhangGuest 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
From: Tina ZhangEnable 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
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 DongCc: 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