| Message ID | 20180330222336.5262-8-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 30, 2018 at 03:23:33PM -0700, José Roberto de Souza wrote: > 'drm/i915/dp: Exit PSR before do a aux channel transaction' cause all > IGT PSR and frontbuffer tracking tests to not be userful. > Those tests depend in reading the calculated CRC value of the > frontbuffer in sink and doing a aux transaction now causes the PSR > to exit. > So any bug in software and hardware buffer tracking will be masked by > this forced PSR exit. > > This is a dirty workaround that makes those tests functional again > at the risk of causing concurrent access in the aux ch registers > while running_igt_psr_tests is set. > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 24 +++++++++++++++++++++++- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_dp.c | 18 ++++++++++++++++++ > 3 files changed, 43 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 1dba2c451255..519f67598d44 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -4732,6 +4732,27 @@ static int i915_drrs_ctl_set(void *data, u64 val) > > DEFINE_SIMPLE_ATTRIBUTE(i915_drrs_ctl_fops, NULL, i915_drrs_ctl_set, "%llu\n"); > > +static int i915_running_igt_psr_tests_get(void *data, u64 *val) > +{ > + struct drm_i915_private *dev_priv = data; > + > + *val = dev_priv->running_igt_psr_tests; > + return 0; > +} > + > +static int i915_running_igt_psr_tests_set(void *data, u64 val) > +{ > + struct drm_i915_private *dev_priv = data; > + > + dev_priv->running_igt_psr_tests = !!val; > + return 0; > +} > + > +DEFINE_SIMPLE_ATTRIBUTE(i915_running_igt_psr_tests_fops, > + i915_running_igt_psr_tests_get, > + i915_running_igt_psr_tests_set, > + "%llu\n"); > + > static const struct drm_info_list i915_debugfs_list[] = { > {"i915_capabilities", i915_capabilities, 0}, > {"i915_gem_objects", i915_gem_object_info, 0}, > @@ -4812,7 +4833,8 @@ static const struct i915_debugfs_files { > {"i915_guc_log_relay", &i915_guc_log_relay_fops}, > {"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops}, > {"i915_ipc_status", &i915_ipc_status_fops}, > - {"i915_drrs_ctl", &i915_drrs_ctl_fops} > + {"i915_drrs_ctl", &i915_drrs_ctl_fops}, > + {"i915_running_igt_psr_tests", &i915_running_igt_psr_tests_fops} we don't need new ops. All we want is to block the exit when running sink_crc tests, so add the blocker wa there... > }; > > int i915_debugfs_register(struct drm_i915_private *dev_priv) > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 41ebb144594e..9e0a5e29f48e 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2110,6 +2110,8 @@ struct drm_i915_private { > > struct i915_pmu pmu; > > + bool running_igt_psr_tests; > + > /* > * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch > * will be rejected. Instead look for a better place. > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index fedee4e7ed24..c655f6c08a02 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1071,8 +1071,20 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp, > */ > static void intel_dp_aux_ch_get(struct intel_dp *intel_dp) > { > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > + struct drm_i915_private *dev_priv = > + to_i915(intel_dig_port->base.base.dev); > + > if (!intel_dp->exit_psr_on_aux_ch_xfer) > return; > + /* > + * HACK: This is a workaround to keep IGT PSR and frontbuffer tracking > + * tests functional, otherwise when IGT request the CRC of the > + * frontbuffer to sink it would cause this function to complete execute > + * and exit PSR. > + */ > + if (dev_priv->running_igt_psr_tests) > + return; > > intel_psr_activate_block_get(intel_dp); > intel_psr_exit(intel_dp, true); > @@ -1085,8 +1097,14 @@ static void intel_dp_aux_ch_get(struct intel_dp *intel_dp) > */ > static void intel_dp_aux_ch_put(struct intel_dp *intel_dp) > { > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > + struct drm_i915_private *dev_priv = > + to_i915(intel_dig_port->base.base.dev); > + > if (!intel_dp->exit_psr_on_aux_ch_xfer) > return; > + if (dev_priv->running_igt_psr_tests) > + return; > > intel_psr_activate_block_put(intel_dp); > /* Usually more than just one aux ch transaction is executed when > -- > 2.16.3 >
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 1dba2c451255..519f67598d44 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -4732,6 +4732,27 @@ static int i915_drrs_ctl_set(void *data, u64 val) DEFINE_SIMPLE_ATTRIBUTE(i915_drrs_ctl_fops, NULL, i915_drrs_ctl_set, "%llu\n"); +static int i915_running_igt_psr_tests_get(void *data, u64 *val) +{ + struct drm_i915_private *dev_priv = data; + + *val = dev_priv->running_igt_psr_tests; + return 0; +} + +static int i915_running_igt_psr_tests_set(void *data, u64 val) +{ + struct drm_i915_private *dev_priv = data; + + dev_priv->running_igt_psr_tests = !!val; + return 0; +} + +DEFINE_SIMPLE_ATTRIBUTE(i915_running_igt_psr_tests_fops, + i915_running_igt_psr_tests_get, + i915_running_igt_psr_tests_set, + "%llu\n"); + static const struct drm_info_list i915_debugfs_list[] = { {"i915_capabilities", i915_capabilities, 0}, {"i915_gem_objects", i915_gem_object_info, 0}, @@ -4812,7 +4833,8 @@ static const struct i915_debugfs_files { {"i915_guc_log_relay", &i915_guc_log_relay_fops}, {"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops}, {"i915_ipc_status", &i915_ipc_status_fops}, - {"i915_drrs_ctl", &i915_drrs_ctl_fops} + {"i915_drrs_ctl", &i915_drrs_ctl_fops}, + {"i915_running_igt_psr_tests", &i915_running_igt_psr_tests_fops} }; int i915_debugfs_register(struct drm_i915_private *dev_priv) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 41ebb144594e..9e0a5e29f48e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2110,6 +2110,8 @@ struct drm_i915_private { struct i915_pmu pmu; + bool running_igt_psr_tests; + /* * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch * will be rejected. Instead look for a better place. diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index fedee4e7ed24..c655f6c08a02 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1071,8 +1071,20 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp, */ static void intel_dp_aux_ch_get(struct intel_dp *intel_dp) { + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); + struct drm_i915_private *dev_priv = + to_i915(intel_dig_port->base.base.dev); + if (!intel_dp->exit_psr_on_aux_ch_xfer) return; + /* + * HACK: This is a workaround to keep IGT PSR and frontbuffer tracking + * tests functional, otherwise when IGT request the CRC of the + * frontbuffer to sink it would cause this function to complete execute + * and exit PSR. + */ + if (dev_priv->running_igt_psr_tests) + return; intel_psr_activate_block_get(intel_dp); intel_psr_exit(intel_dp, true); @@ -1085,8 +1097,14 @@ static void intel_dp_aux_ch_get(struct intel_dp *intel_dp) */ static void intel_dp_aux_ch_put(struct intel_dp *intel_dp) { + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); + struct drm_i915_private *dev_priv = + to_i915(intel_dig_port->base.base.dev); + if (!intel_dp->exit_psr_on_aux_ch_xfer) return; + if (dev_priv->running_igt_psr_tests) + return; intel_psr_activate_block_put(intel_dp); /* Usually more than just one aux ch transaction is executed when
'drm/i915/dp: Exit PSR before do a aux channel transaction' cause all IGT PSR and frontbuffer tracking tests to not be userful. Those tests depend in reading the calculated CRC value of the frontbuffer in sink and doing a aux transaction now causes the PSR to exit. So any bug in software and hardware buffer tracking will be masked by this forced PSR exit. This is a dirty workaround that makes those tests functional again at the risk of causing concurrent access in the aux ch registers while running_igt_psr_tests is set. Signed-off-by: José Roberto de Souza <jose.souza@intel.com> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 24 +++++++++++++++++++++++- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/intel_dp.c | 18 ++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-)