| Message ID | 1449072510-1721-1-git-send-email-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Dec 02, 2015 at 08:08:30AM -0800, Rodrigo Vivi wrote: > With a reliable frontbuffer tracking and all instability corner cases > solved let's re-enabled PSR by default on all supported platforms. > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Hm, one thing we're missing here still I think is that right now there's not yet a BAT for psr. At least I didn't spot anything in the CI overview. Is there a super-basic PSR testcase (just checking that we go into psr self-refresh is imo good enough), similar to what we have with runtime PM? Also I guess we need to get the dp aux series from you in first. With BAT and outstanding series merged: Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Awesome work! Cheers, Daniel > --- > drivers/gpu/drm/i915/i915_params.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > index 835d609..461c326 100644 > --- a/drivers/gpu/drm/i915/i915_params.c > +++ b/drivers/gpu/drm/i915/i915_params.c > @@ -37,7 +37,7 @@ struct i915_params i915 __read_mostly = { > .enable_execlists = -1, > .enable_hangcheck = true, > .enable_ppgtt = -1, > - .enable_psr = 0, > + .enable_psr = 1, > .preliminary_hw_support = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT), > .disable_power_well = -1, > .enable_ips = 1, > @@ -126,7 +126,7 @@ MODULE_PARM_DESC(enable_execlists, > "(-1=auto [default], 0=disabled, 1=enabled)"); > > module_param_named_unsafe(enable_psr, i915.enable_psr, int, 0600); > -MODULE_PARM_DESC(enable_psr, "Enable PSR (default: false)"); > +MODULE_PARM_DESC(enable_psr, "Enable PSR (default: true)"); > > module_param_named_unsafe(preliminary_hw_support, i915.preliminary_hw_support, int, 0600); > MODULE_PARM_DESC(preliminary_hw_support, > -- > 2.4.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, 2015-12-04 at 16:31 +0100, Daniel Vetter wrote: > On Wed, Dec 02, 2015 at 08:08:30AM -0800, Rodrigo Vivi wrote: > > With a reliable frontbuffer tracking and all instability corner > > cases > > solved let's re-enabled PSR by default on all supported platforms. > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Hm, one thing we're missing here still I think is that right now > there's > not yet a BAT for psr. Yes, agree. > At least I didn't spot anything in the CI overview. > Is there a super-basic PSR testcase (just checking that we go into > psr > self-refresh is imo good enough), similar to what we have with > runtime PM? we had that in the past but in the end I removed. Maybe get it back and call basic. But also we could have few cases like page_flip, mmap_gtt and mmap_cpu as BAT as well. What do you think? > > Also I guess we need to get the dp aux series from you in first. Actually I believe the only required patch there is that one that check for message size. Would you accept split that patch from that series for now with a FIXME? I promisse I won't give up on killing intel_dp_dpcd_read_wake ;) > With BAT > and outstanding series merged: > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Awesome work! > > Cheers, Daniel Thank you very much! Rodrigo. > > --- > > drivers/gpu/drm/i915/i915_params.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_params.c > > b/drivers/gpu/drm/i915/i915_params.c > > index 835d609..461c326 100644 > > --- a/drivers/gpu/drm/i915/i915_params.c > > +++ b/drivers/gpu/drm/i915/i915_params.c > > @@ -37,7 +37,7 @@ struct i915_params i915 __read_mostly = { > > .enable_execlists = -1, > > .enable_hangcheck = true, > > .enable_ppgtt = -1, > > - .enable_psr = 0, > > + .enable_psr = 1, > > .preliminary_hw_support = > > IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT), > > .disable_power_well = -1, > > .enable_ips = 1, > > @@ -126,7 +126,7 @@ MODULE_PARM_DESC(enable_execlists, > > "(-1=auto [default], 0=disabled, 1=enabled)"); > > > > module_param_named_unsafe(enable_psr, i915.enable_psr, int, 0600); > > -MODULE_PARM_DESC(enable_psr, "Enable PSR (default: false)"); > > +MODULE_PARM_DESC(enable_psr, "Enable PSR (default: true)"); > > > > module_param_named_unsafe(preliminary_hw_support, > > i915.preliminary_hw_support, int, 0600); > > MODULE_PARM_DESC(preliminary_hw_support, > > -- > > 2.4.3 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Fri, Dec 04, 2015 at 04:22:20PM +0000, Vivi, Rodrigo wrote: > On Fri, 2015-12-04 at 16:31 +0100, Daniel Vetter wrote: > > On Wed, Dec 02, 2015 at 08:08:30AM -0800, Rodrigo Vivi wrote: > > > With a reliable frontbuffer tracking and all instability corner > > > cases > > > solved let's re-enabled PSR by default on all supported platforms. > > > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > > Hm, one thing we're missing here still I think is that right now > > there's > > not yet a BAT for psr. > > Yes, agree. > > > At least I didn't spot anything in the CI overview. > > Is there a super-basic PSR testcase (just checking that we go into > > psr > > self-refresh is imo good enough), similar to what we have with > > runtime PM? > > we had that in the past but in the end I removed. Maybe get it back and > call basic. But also we could have few cases like page_flip, mmap_gtt > and mmap_cpu as BAT as well. What do you think? BAT is already taking a while to run, and any modeset tests we add take away more time. I think just a basic one should be good enough for BAT. Imo BAT really should be just the "is this total crap?" test. We will still run full igt (at least that's my goal) on any patch that goes in, with the goal to run all the fast tests (less than 1 minute or so) pre-merge. In this glorious future BAT will only decide whether more tests should even be started. > > Also I guess we need to get the dp aux series from you in first. > > Actually I believe the only required patch there is that one that check > for message size. Would you accept split that patch from that series > for now with a FIXME? > I promisse I won't give up on killing intel_dp_dpcd_read_wake ;) Makes sense I'd say. Cheers, Daniel > > > With BAT > > and outstanding series merged: > > > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > Awesome work! > > > > Cheers, Daniel > > Thank you very much! > Rodrigo. > > > > --- > > > drivers/gpu/drm/i915/i915_params.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_params.c > > > b/drivers/gpu/drm/i915/i915_params.c > > > index 835d609..461c326 100644 > > > --- a/drivers/gpu/drm/i915/i915_params.c > > > +++ b/drivers/gpu/drm/i915/i915_params.c > > > @@ -37,7 +37,7 @@ struct i915_params i915 __read_mostly = { > > > .enable_execlists = -1, > > > .enable_hangcheck = true, > > > .enable_ppgtt = -1, > > > - .enable_psr = 0, > > > + .enable_psr = 1, > > > .preliminary_hw_support = > > > IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT), > > > .disable_power_well = -1, > > > .enable_ips = 1, > > > @@ -126,7 +126,7 @@ MODULE_PARM_DESC(enable_execlists, > > > "(-1=auto [default], 0=disabled, 1=enabled)"); > > > > > > module_param_named_unsafe(enable_psr, i915.enable_psr, int, 0600); > > > -MODULE_PARM_DESC(enable_psr, "Enable PSR (default: false)"); > > > +MODULE_PARM_DESC(enable_psr, "Enable PSR (default: true)"); > > > > > > module_param_named_unsafe(preliminary_hw_support, > > > i915.preliminary_hw_support, int, 0600); > > > MODULE_PARM_DESC(preliminary_hw_support, > > > -- > > > 2.4.3 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > >
On Fri, 2015-12-04 at 17:54 +0100, Daniel Vetter wrote: > On Fri, Dec 04, 2015 at 04:22:20PM +0000, Vivi, Rodrigo wrote: > > On Fri, 2015-12-04 at 16:31 +0100, Daniel Vetter wrote: > > > On Wed, Dec 02, 2015 at 08:08:30AM -0800, Rodrigo Vivi wrote: > > > > With a reliable frontbuffer tracking and all instability corner > > > > cases > > > > solved let's re-enabled PSR by default on all supported > > > > platforms. > > > > > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > > > > Hm, one thing we're missing here still I think is that right now > > > there's > > > not yet a BAT for psr. > > > > Yes, agree. > > > > > At least I didn't spot anything in the CI overview. > > > Is there a super-basic PSR testcase (just checking that we go > > > into > > > psr > > > self-refresh is imo good enough), similar to what we have with > > > runtime PM? > > > > we had that in the past but in the end I removed. Maybe get it back > > and > > call basic. But also we could have few cases like page_flip, > > mmap_gtt > > and mmap_cpu as BAT as well. What do you think? > > BAT is already taking a while to run, and any modeset tests we add > take > away more time. I think just a basic one should be good enough for > BAT. > Imo BAT really should be just the "is this total crap?" test. We will > still run full igt (at least that's my goal) on any patch that goes > in, > with the goal to run all the fast tests (less than 1 minute or so) > pre-merge. In this glorious future BAT will only decide whether more > tests > should even be started. Makes sense. > > > > Also I guess we need to get the dp aux series from you in first. > > > > Actually I believe the only required patch there is that one that > > check > > for message size. Would you accept split that patch from that > > series > > for now with a FIXME? > > I promisse I won't give up on killing intel_dp_dpcd_read_wake ;) > > Makes sense I'd say. Thanks! > > Cheers, Daniel > > > > > With BAT > > > and outstanding series merged: > > > > > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > > > Awesome work! > > > > > > Cheers, Daniel > > > > Thank you very much! > > Rodrigo. > > > > > > --- > > > > drivers/gpu/drm/i915/i915_params.c | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_params.c > > > > b/drivers/gpu/drm/i915/i915_params.c > > > > index 835d609..461c326 100644 > > > > --- a/drivers/gpu/drm/i915/i915_params.c > > > > +++ b/drivers/gpu/drm/i915/i915_params.c > > > > @@ -37,7 +37,7 @@ struct i915_params i915 __read_mostly = { > > > > .enable_execlists = -1, > > > > .enable_hangcheck = true, > > > > .enable_ppgtt = -1, > > > > - .enable_psr = 0, > > > > + .enable_psr = 1, > > > > .preliminary_hw_support = > > > > IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT), > > > > .disable_power_well = -1, > > > > .enable_ips = 1, > > > > @@ -126,7 +126,7 @@ MODULE_PARM_DESC(enable_execlists, > > > > "(-1=auto [default], 0=disabled, 1=enabled)"); > > > > > > > > module_param_named_unsafe(enable_psr, i915.enable_psr, int, > > > > 0600); > > > > -MODULE_PARM_DESC(enable_psr, "Enable PSR (default: false)"); > > > > +MODULE_PARM_DESC(enable_psr, "Enable PSR (default: true)"); > > > > > > > > module_param_named_unsafe(preliminary_hw_support, > > > > i915.preliminary_hw_support, int, 0600); > > > > MODULE_PARM_DESC(preliminary_hw_support, > > > > -- > > > > 2.4.3 > > > > > > > > _______________________________________________ > > > > Intel-gfx mailing list > > > > Intel-gfx@lists.freedesktop.org > > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > >
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 835d609..461c326 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -37,7 +37,7 @@ struct i915_params i915 __read_mostly = { .enable_execlists = -1, .enable_hangcheck = true, .enable_ppgtt = -1, - .enable_psr = 0, + .enable_psr = 1, .preliminary_hw_support = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT), .disable_power_well = -1, .enable_ips = 1, @@ -126,7 +126,7 @@ MODULE_PARM_DESC(enable_execlists, "(-1=auto [default], 0=disabled, 1=enabled)"); module_param_named_unsafe(enable_psr, i915.enable_psr, int, 0600); -MODULE_PARM_DESC(enable_psr, "Enable PSR (default: false)"); +MODULE_PARM_DESC(enable_psr, "Enable PSR (default: true)"); module_param_named_unsafe(preliminary_hw_support, i915.preliminary_hw_support, int, 0600); MODULE_PARM_DESC(preliminary_hw_support,
With a reliable frontbuffer tracking and all instability corner cases solved let's re-enabled PSR by default on all supported platforms. Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/i915_params.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)