Re: [Intel-gfx] [PATCH 11/11] drm/i915: Enable PSR by default.
On Sat, Jul 12, 2014 at 10:48:30PM +0200, Daniel Vetter wrote: On Sat, Jul 12, 2014 at 8:55 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Sat, Jul 12, 2014 at 08:15:42PM +0200, Daniel Vetter wrote: On Sat, Jul 12, 2014 at 7:51 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Sat, Jul 12, 2014 at 07:14:33PM +0200, Daniel Vetter wrote: On Sat, Jul 12, 2014 at 2:02 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: But PSR is definitely an output property... Please-dont-touch-me is indeed a plane property. Well I guess we also want this for fbc, and fbc isn't an output property. I guess in the end it doesn't really matter that much as long as it's there, but traditional userspace exposes all output properties to userspace (on X), hence why I think we should hide it a bit ;-) Hence why I want PSR as an output property. I want to see the status exposed in xrandr. Hm, why that? I've thought we only want this to give hints to the compositor/ddx? An interesting factoid to present to the user. Also makes it easy for me to check everything works in X. Hm not sure we want users to know this stuff. Next they ask to adjust it and then we have the same mess as with kernel options ;-) But I don't care strongly enough really either way. In-kernel I still think we want to keep this on the crtc so that fbc and psr work the same way. The connector property would then just chase the connected crtc to read out the property. It can't be crtc as it is an connector properrty... And it is immutable. I think it will be one of those useful things that people would like to check infrequently to make sure everything is functioning as intended (like powertop). I think there may be a few properties like this we can expose. And if people ask why PSR isn't active, then we should do a better job at making sure it stays enabled. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/11] drm/i915: Enable PSR by default.
On Sun, Jul 13, 2014 at 8:30 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Sat, Jul 12, 2014 at 10:48:30PM +0200, Daniel Vetter wrote: On Sat, Jul 12, 2014 at 8:55 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Sat, Jul 12, 2014 at 08:15:42PM +0200, Daniel Vetter wrote: On Sat, Jul 12, 2014 at 7:51 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Sat, Jul 12, 2014 at 07:14:33PM +0200, Daniel Vetter wrote: On Sat, Jul 12, 2014 at 2:02 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: But PSR is definitely an output property... Please-dont-touch-me is indeed a plane property. Well I guess we also want this for fbc, and fbc isn't an output property. I guess in the end it doesn't really matter that much as long as it's there, but traditional userspace exposes all output properties to userspace (on X), hence why I think we should hide it a bit ;-) Hence why I want PSR as an output property. I want to see the status exposed in xrandr. Hm, why that? I've thought we only want this to give hints to the compositor/ddx? An interesting factoid to present to the user. Also makes it easy for me to check everything works in X. Hm not sure we want users to know this stuff. Next they ask to adjust it and then we have the same mess as with kernel options ;-) But I don't care strongly enough really either way. In-kernel I still think we want to keep this on the crtc so that fbc and psr work the same way. The connector property would then just chase the connected crtc to read out the property. It can't be crtc as it is an connector properrty... And it is immutable. I think it will be one of those useful things that people would like to check infrequently to make sure everything is functioning as intended (like powertop). I think there may be a few properties like this we can expose. And if people ask why PSR isn't active, then we should do a better job at making sure it stays enabled. Oh, you mean just a psr property? I've thought in general terms of discouraging frontbuffer rendering and included fbc. A simple has_self_refresh_display (don't want to exclude dsi) is obviously a connector thing. But I thought you want to use that as a should I flip or can I frontbuffer render hint, and I guessed that fbc should be part of that. At least once we switched it to the sw tracking. -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 http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/11] drm/i915: Enable PSR by default.
On Sun, Jul 13, 2014 at 12:16:20PM +0200, Daniel Vetter wrote: On Sun, Jul 13, 2014 at 8:30 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Sat, Jul 12, 2014 at 10:48:30PM +0200, Daniel Vetter wrote: On Sat, Jul 12, 2014 at 8:55 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Sat, Jul 12, 2014 at 08:15:42PM +0200, Daniel Vetter wrote: On Sat, Jul 12, 2014 at 7:51 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Sat, Jul 12, 2014 at 07:14:33PM +0200, Daniel Vetter wrote: On Sat, Jul 12, 2014 at 2:02 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: But PSR is definitely an output property... Please-dont-touch-me is indeed a plane property. Well I guess we also want this for fbc, and fbc isn't an output property. I guess in the end it doesn't really matter that much as long as it's there, but traditional userspace exposes all output properties to userspace (on X), hence why I think we should hide it a bit ;-) Hence why I want PSR as an output property. I want to see the status exposed in xrandr. Hm, why that? I've thought we only want this to give hints to the compositor/ddx? An interesting factoid to present to the user. Also makes it easy for me to check everything works in X. Hm not sure we want users to know this stuff. Next they ask to adjust it and then we have the same mess as with kernel options ;-) But I don't care strongly enough really either way. In-kernel I still think we want to keep this on the crtc so that fbc and psr work the same way. The connector property would then just chase the connected crtc to read out the property. It can't be crtc as it is an connector properrty... And it is immutable. I think it will be one of those useful things that people would like to check infrequently to make sure everything is functioning as intended (like powertop). I think there may be a few properties like this we can expose. And if people ask why PSR isn't active, then we should do a better job at making sure it stays enabled. Oh, you mean just a psr property? I've thought in general terms of discouraging frontbuffer rendering and included fbc. A simple has_self_refresh_display (don't want to exclude dsi) is obviously a connector thing. But I thought you want to use that as a should I flip or can I frontbuffer render hint, and I guessed that fbc should be part of that. At least once we switched it to the sw tracking. Yes, I was after both. But at the moment, I am interested in having a user-visible flag for showing that your fancy low power display is working. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/11] drm/i915: Enable PSR by default.
On Fri, Jul 11, 2014 at 10:30:19AM -0700, Rodrigo Vivi wrote:
Panel Self Refresh is an eDP power saving feature specified by VESA's eDP
v1.3,
that allows some panel componets to shutdown while you still see static
images on
the screen. Besides being supported on the platform it must be supported by
the
eDP panel itself.
Now that we have the propper frontbuffer tracking support and correct locks
on place
we can enabled this feature by default.
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
Yay!
Entire series pulled in, thanks a lot for the testing and review.
Besides byt psr the big chunk is now polishing the testcase. Iirc I've
written down a detailed description of the task in internal jira and
assigned it to you, right?
I want to have the tests before we enable byt psr to make sure we really
catch all the regressions that might crop up.
Thanks, 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 8145729..bbdee21 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_fbc = -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,
@@ -118,7 +118,7 @@ MODULE_PARM_DESC(enable_ppgtt,
(-1=auto [default], 0=disabled, 1=aliasing, 2=full));
module_param_named(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(preliminary_hw_support, i915.preliminary_hw_support, int,
0600);
MODULE_PARM_DESC(preliminary_hw_support,
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/11] drm/i915: Enable PSR by default.
On Sat, Jul 12, 2014 at 12:05:13PM +0200, Daniel Vetter wrote: On Fri, Jul 11, 2014 at 10:30:19AM -0700, Rodrigo Vivi wrote: Panel Self Refresh is an eDP power saving feature specified by VESA's eDP v1.3, that allows some panel componets to shutdown while you still see static images on the screen. Besides being supported on the platform it must be supported by the eDP panel itself. Now that we have the propper frontbuffer tracking support and correct locks on place we can enabled this feature by default. Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Yay! Entire series pulled in, thanks a lot for the testing and review. Is there any chance we can have the output property to know when PSR is available and active on the eDP? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/11] drm/i915: Enable PSR by default.
On Sat, Jul 12, 2014 at 12:42 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Sat, Jul 12, 2014 at 12:05:13PM +0200, Daniel Vetter wrote: On Fri, Jul 11, 2014 at 10:30:19AM -0700, Rodrigo Vivi wrote: Panel Self Refresh is an eDP power saving feature specified by VESA's eDP v1.3, that allows some panel componets to shutdown while you still see static images on the screen. Besides being supported on the platform it must be supported by the eDP panel itself. Now that we have the propper frontbuffer tracking support and correct locks on place we can enabled this feature by default. Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Yay! Entire series pulled in, thanks a lot for the testing and review. Is there any chance we can have the output property to know when PSR is available and active on the eDP? Oh right, forgotten about this. I guess now that the hw stuff is there we can resurrect this. And with the universal planes support we could move the prefer backbuffer rendering hint to planes even where it imo belongs (since e.g. fbc is on the plane). Or maybe we don't care about that level of granularity and just have one per pipe. -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 http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/11] drm/i915: Enable PSR by default.
On Sat, Jul 12, 2014 at 01:20:10PM +0200, Daniel Vetter wrote: On Sat, Jul 12, 2014 at 12:42 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Sat, Jul 12, 2014 at 12:05:13PM +0200, Daniel Vetter wrote: On Fri, Jul 11, 2014 at 10:30:19AM -0700, Rodrigo Vivi wrote: Panel Self Refresh is an eDP power saving feature specified by VESA's eDP v1.3, that allows some panel componets to shutdown while you still see static images on the screen. Besides being supported on the platform it must be supported by the eDP panel itself. Now that we have the propper frontbuffer tracking support and correct locks on place we can enabled this feature by default. Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Yay! Entire series pulled in, thanks a lot for the testing and review. Is there any chance we can have the output property to know when PSR is available and active on the eDP? Oh right, forgotten about this. I guess now that the hw stuff is there we can resurrect this. And with the universal planes support we could move the prefer backbuffer rendering hint to planes even where it imo belongs (since e.g. fbc is on the plane). Or maybe we don't care about that level of granularity and just have one per pipe. But PSR is definitely an output property... Please-dont-touch-me is indeed a plane property. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/11] drm/i915: Enable PSR by default.
On Sat, Jul 12, 2014 at 2:02 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: But PSR is definitely an output property... Please-dont-touch-me is indeed a plane property. Well I guess we also want this for fbc, and fbc isn't an output property. I guess in the end it doesn't really matter that much as long as it's there, but traditional userspace exposes all output properties to userspace (on X), hence why I think we should hide it a bit ;-) -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 http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/11] drm/i915: Enable PSR by default.
On Sat, Jul 12, 2014 at 07:14:33PM +0200, Daniel Vetter wrote: On Sat, Jul 12, 2014 at 2:02 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: But PSR is definitely an output property... Please-dont-touch-me is indeed a plane property. Well I guess we also want this for fbc, and fbc isn't an output property. I guess in the end it doesn't really matter that much as long as it's there, but traditional userspace exposes all output properties to userspace (on X), hence why I think we should hide it a bit ;-) Hence why I want PSR as an output property. I want to see the status exposed in xrandr. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/11] drm/i915: Enable PSR by default.
On Sat, Jul 12, 2014 at 7:51 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Sat, Jul 12, 2014 at 07:14:33PM +0200, Daniel Vetter wrote: On Sat, Jul 12, 2014 at 2:02 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: But PSR is definitely an output property... Please-dont-touch-me is indeed a plane property. Well I guess we also want this for fbc, and fbc isn't an output property. I guess in the end it doesn't really matter that much as long as it's there, but traditional userspace exposes all output properties to userspace (on X), hence why I think we should hide it a bit ;-) Hence why I want PSR as an output property. I want to see the status exposed in xrandr. Hm, why that? I've thought we only want this to give hints to the compositor/ddx? -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 http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/11] drm/i915: Enable PSR by default.
On Sat, Jul 12, 2014 at 08:15:42PM +0200, Daniel Vetter wrote: On Sat, Jul 12, 2014 at 7:51 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Sat, Jul 12, 2014 at 07:14:33PM +0200, Daniel Vetter wrote: On Sat, Jul 12, 2014 at 2:02 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: But PSR is definitely an output property... Please-dont-touch-me is indeed a plane property. Well I guess we also want this for fbc, and fbc isn't an output property. I guess in the end it doesn't really matter that much as long as it's there, but traditional userspace exposes all output properties to userspace (on X), hence why I think we should hide it a bit ;-) Hence why I want PSR as an output property. I want to see the status exposed in xrandr. Hm, why that? I've thought we only want this to give hints to the compositor/ddx? An interesting factoid to present to the user. Also makes it easy for me to check everything works in X. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/11] drm/i915: Enable PSR by default.
On Sat, Jul 12, 2014 at 8:55 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Sat, Jul 12, 2014 at 08:15:42PM +0200, Daniel Vetter wrote: On Sat, Jul 12, 2014 at 7:51 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Sat, Jul 12, 2014 at 07:14:33PM +0200, Daniel Vetter wrote: On Sat, Jul 12, 2014 at 2:02 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: But PSR is definitely an output property... Please-dont-touch-me is indeed a plane property. Well I guess we also want this for fbc, and fbc isn't an output property. I guess in the end it doesn't really matter that much as long as it's there, but traditional userspace exposes all output properties to userspace (on X), hence why I think we should hide it a bit ;-) Hence why I want PSR as an output property. I want to see the status exposed in xrandr. Hm, why that? I've thought we only want this to give hints to the compositor/ddx? An interesting factoid to present to the user. Also makes it easy for me to check everything works in X. Hm not sure we want users to know this stuff. Next they ask to adjust it and then we have the same mess as with kernel options ;-) But I don't care strongly enough really either way. In-kernel I still think we want to keep this on the crtc so that fbc and psr work the same way. The connector property would then just chase the connected crtc to read out the property. -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 http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/11] drm/i915: Enable PSR by default.
Panel Self Refresh is an eDP power saving feature specified by VESA's eDP v1.3,
that allows some panel componets to shutdown while you still see static images
on
the screen. Besides being supported on the platform it must be supported by the
eDP panel itself.
Now that we have the propper frontbuffer tracking support and correct locks on
place
we can enabled this feature by default.
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
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 8145729..bbdee21 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_fbc = -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,
@@ -118,7 +118,7 @@ MODULE_PARM_DESC(enable_ppgtt,
(-1=auto [default], 0=disabled, 1=aliasing, 2=full));
module_param_named(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(preliminary_hw_support, i915.preliminary_hw_support, int,
0600);
MODULE_PARM_DESC(preliminary_hw_support,
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
