On Mon, 2018-11-12 at 11:17 +0100, Maarten Lankhorst wrote: > Op 09-11-18 om 21:20 schreef José Roberto de Souza: > > If panel supports DRRS and PSR and if driver is loaded without PSR > > enabled, driver will enable DRRS as expected but if PSR is enabled > > by > > debugfs latter it will keep PSR and DRRS enabled causing possible > > problems as DRRS will lower the refresh rate while PSR enabled. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108341 > > Cc: Maarten Lankhorst <[email protected]> > > Cc: Dhinakaran Pandiyan <[email protected]> > > Signed-off-by: José Roberto de Souza <[email protected]> > > --- > > drivers/gpu/drm/i915/intel_psr.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > > b/drivers/gpu/drm/i915/intel_psr.c > > index 853e3f1370a0..bfc6a08b5cf4 100644 > > --- a/drivers/gpu/drm/i915/intel_psr.c > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > @@ -904,8 +904,11 @@ int intel_psr_set_debugfs_mode(struct > > drm_i915_private *dev_priv, > > > > intel_psr_irq_control(dev_priv, dev_priv->psr.debug); > > > > - if (dev_priv->psr.prepared && enable) > > + if (dev_priv->psr.prepared && enable) { > > + if (crtc_state) > > + intel_edp_drrs_disable(dp, crtc_state); > > intel_psr_enable_locked(dev_priv, crtc_state); > > + } > > > > mutex_unlock(&dev_priv->psr.lock); > > return ret; > > I've considered this, but I thought it was a feature, not a bug. It's > a pain to track > how we handle this as intended. > > kms_frontbuffer_tracking is also controlling DRRS during the test, so > perhaps simply > fix the test? > > It seems the no_drrs test simply checks that if PSR is enabled, we > don't have drrs > enabled. We probably care about the default configuration, so I would > simply disable > the pipe, update the PSR flag, and then start running the tests. Else > the only thing > we test is that debugfs disables DRRS. Not that the default modeset > path prevents > PSR and DRRS simultaneously.
Yeah, I think we should force a modeset from debugs to test the default
modeset path, fix the tests I think is a bad idea as it could leave
some test cases unhandled.
Also looking at DRRS it looks complete broken for page flips, it would
kept set to DRRS_LOW forever.
>
> ~Maarten
>
> Maybe something like below?
>
> Perhaps move the drrs manipulation functions from
> kms_frontbuffer_tracking to lib/kms_psr.c
>
> ----8<-------
> diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> index 9767f475bf23..ffc356df06ce 100644
> --- a/tests/kms_psr.c
> +++ b/tests/kms_psr.c
> @@ -414,9 +414,6 @@ int main(int argc, char *argv[])
> kmstest_set_vt_graphics_mode();
> data.devid = intel_get_drm_devid(data.drm_fd);
>
> - if (!data.with_psr_disabled)
> - psr_enable(data.debugfs_fd);
> -
> igt_require_f(sink_support(&data),
> "Sink does not support PSR\n");
>
> @@ -428,18 +425,25 @@ int main(int argc, char *argv[])
> }
>
> igt_subtest("basic") {
> - setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> - igt_assert(psr_wait_entry_if_enabled(&data));
> - test_cleanup(&data);
> - }
> + /* Disable display to get a default setup. */
> + igt_display_commit2(&data.display,
> data.display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
> +
> + if (!data.with_psr_disabled)
> + psr_enable(data.debugfs_fd);
>
> - igt_subtest("no_drrs") {
> setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> igt_assert(psr_wait_entry_if_enabled(&data));
> igt_assert(drrs_disabled(&data));
> test_cleanup(&data);
> }
>
> + igt_fixture {
> + drrs_disable();
> +
> + if (!data.with_psr_disabled)
> + psr_enable(data.debugfs_fd);
> + }
> +
> for (op = PAGE_FLIP; op <= RENDER; op++) {
> igt_subtest_f("primary_%s", op_str(op)) {
> data.op = op;
>
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
