All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm: add client cap to expose low power modes
@ 2020-10-21  6:54 Ken Huang
  2020-10-21  7:12 ` Maarten Lankhorst
  2020-10-21  7:40 ` Simon Ser
  0 siblings, 2 replies; 10+ messages in thread
From: Ken Huang @ 2020-10-21  6:54 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Daniel Vetter, dri-devel
  Cc: Ken Huang, Adrian Salido

From: Adrian Salido <salidoa@google.com>

Some displays may support Low Power modes, however, these modes may
require special handling as they would usually require lower OPR
content on framebuffers. Add a drm mode flag to specify display
capability to support low power mode, and add a drm client cap for
it. If a client doesn't support the capability, that will filter it
out for the client.

Signed-off-by: Adrian Salido <salidoa@google.com>
Signed-off-by: Ken Huang <kenbshuang@google.com>
---
 drivers/gpu/drm/drm_connector.c |  4 ++++
 drivers/gpu/drm/drm_ioctl.c     |  5 +++++
 include/drm/drm_file.h          |  7 +++++++
 include/uapi/drm/drm.h          | 10 ++++++++++
 include/uapi/drm/drm_mode.h     |  3 +++
 5 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 717c4e7271b0..46a29b156ffa 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2318,6 +2318,10 @@ drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
 		}
 	}
 
+	if (!file_priv->low_power_modes_allowed &&
+	    (mode->flags & DRM_MODE_FLAG_LOW_POWER))
+		return false;
+
 	return true;
 }
 
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 789ee65ac1f5..e7e025698b3b 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -362,6 +362,11 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
 			return -EINVAL;
 		file_priv->writeback_connectors = req->value;
 		break;
+	case DRM_CLIENT_CAP_LOW_POWER_MODES:
+		if (req->value > 1)
+			return -EINVAL;
+		file_priv->low_power_modes_allowed = req->value;
+		break;
 	default:
 		return -EINVAL;
 	}
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index 716990bace10..2fa66c32f066 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -212,6 +212,13 @@ struct drm_file {
 	 */
 	bool was_master;
 
+	/**
+	 * @low_power_modes_allowed:
+	 *
+	 * True if the client understands how to work with low power modes
+	 */
+	bool low_power_modes_allowed;
+
 	/**
 	 * @is_master:
 	 *
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 808b48a93330..12f39a628bb5 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -698,6 +698,16 @@ struct drm_get_cap {
  */
 #define DRM_CLIENT_CAP_WRITEBACK_CONNECTORS	5
 
+/**
+ * DRM_CLIENT_CAP_LOW_POWER_MODES
+ *
+ * If set to 1, the DRM core will expose modes that support Lower Power at the
+ * potential cost of reduced fidelity. Special care must be taken by clients
+ * that work with these modes, (ex. framebuffer contents are expected to
+ * have reduced OPRs)
+ */
+#define DRM_CLIENT_CAP_LOW_POWER_MODES	6
+
 /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
 struct drm_set_client_cap {
 	__u64 capability;
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 863eda048265..71137280b1e6 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -124,6 +124,8 @@ extern "C" {
 #define  DRM_MODE_FLAG_PIC_AR_256_135 \
 			(DRM_MODE_PICTURE_ASPECT_256_135<<19)
 
+#define  DRM_MODE_FLAG_LOW_POWER		(1<<23)
+
 #define  DRM_MODE_FLAG_ALL	(DRM_MODE_FLAG_PHSYNC |		\
 				 DRM_MODE_FLAG_NHSYNC |		\
 				 DRM_MODE_FLAG_PVSYNC |		\
@@ -136,6 +138,7 @@ extern "C" {
 				 DRM_MODE_FLAG_HSKEW |		\
 				 DRM_MODE_FLAG_DBLCLK |		\
 				 DRM_MODE_FLAG_CLKDIV2 |	\
+				 DRM_MODE_FLAG_LOW_POWER |	\
 				 DRM_MODE_FLAG_3D_MASK)
 
 /* DPMS flags */
-- 
2.29.0.rc1.297.gfa9743e501-goog

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm: add client cap to expose low power modes
  2020-10-21  6:54 [PATCH] drm: add client cap to expose low power modes Ken Huang
@ 2020-10-21  7:12 ` Maarten Lankhorst
  2020-10-21  7:40 ` Simon Ser
  1 sibling, 0 replies; 10+ messages in thread
From: Maarten Lankhorst @ 2020-10-21  7:12 UTC (permalink / raw)
  To: Ken Huang, Maxime Ripard, Thomas Zimmermann, Daniel Vetter, dri-devel
  Cc: Adrian Salido

Op 21-10-2020 om 08:54 schreef Ken Huang:
> From: Adrian Salido <salidoa@google.com>
>
> Some displays may support Low Power modes, however, these modes may
> require special handling as they would usually require lower OPR
> content on framebuffers. Add a drm mode flag to specify display
> capability to support low power mode, and add a drm client cap for
> it. If a client doesn't support the capability, that will filter it
> out for the client.
>
> Signed-off-by: Adrian Salido <salidoa@google.com>
> Signed-off-by: Ken Huang <kenbshuang@google.com>
> ---
>  drivers/gpu/drm/drm_connector.c |  4 ++++
>  drivers/gpu/drm/drm_ioctl.c     |  5 +++++
>  include/drm/drm_file.h          |  7 +++++++
>  include/uapi/drm/drm.h          | 10 ++++++++++
>  include/uapi/drm/drm_mode.h     |  3 +++
>  5 files changed, 29 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 717c4e7271b0..46a29b156ffa 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -2318,6 +2318,10 @@ drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
>  		}
>  	}
>  
> +	if (!file_priv->low_power_modes_allowed &&
> +	    (mode->flags & DRM_MODE_FLAG_LOW_POWER))
> +		return false;
> +
>  	return true;
>  }
>  
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 789ee65ac1f5..e7e025698b3b 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -362,6 +362,11 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
>  			return -EINVAL;
>  		file_priv->writeback_connectors = req->value;
>  		break;
> +	case DRM_CLIENT_CAP_LOW_POWER_MODES:
> +		if (req->value > 1)
> +			return -EINVAL;
> +		file_priv->low_power_modes_allowed = req->value;
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> index 716990bace10..2fa66c32f066 100644
> --- a/include/drm/drm_file.h
> +++ b/include/drm/drm_file.h
> @@ -212,6 +212,13 @@ struct drm_file {
>  	 */
>  	bool was_master;
>  
> +	/**
> +	 * @low_power_modes_allowed:
> +	 *
> +	 * True if the client understands how to work with low power modes
> +	 */
> +	bool low_power_modes_allowed;
> +
>  	/**
>  	 * @is_master:
>  	 *
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 808b48a93330..12f39a628bb5 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -698,6 +698,16 @@ struct drm_get_cap {
>   */
>  #define DRM_CLIENT_CAP_WRITEBACK_CONNECTORS	5
>  
> +/**
> + * DRM_CLIENT_CAP_LOW_POWER_MODES
> + *
> + * If set to 1, the DRM core will expose modes that support Lower Power at the
> + * potential cost of reduced fidelity. Special care must be taken by clients
> + * that work with these modes, (ex. framebuffer contents are expected to
> + * have reduced OPRs)
> + */
> +#define DRM_CLIENT_CAP_LOW_POWER_MODES	6
> +
>  /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
>  struct drm_set_client_cap {
>  	__u64 capability;
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 863eda048265..71137280b1e6 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -124,6 +124,8 @@ extern "C" {
>  #define  DRM_MODE_FLAG_PIC_AR_256_135 \
>  			(DRM_MODE_PICTURE_ASPECT_256_135<<19)
>  
> +#define  DRM_MODE_FLAG_LOW_POWER		(1<<23)
> +
>  #define  DRM_MODE_FLAG_ALL	(DRM_MODE_FLAG_PHSYNC |		\
>  				 DRM_MODE_FLAG_NHSYNC |		\
>  				 DRM_MODE_FLAG_PVSYNC |		\
> @@ -136,6 +138,7 @@ extern "C" {
>  				 DRM_MODE_FLAG_HSKEW |		\
>  				 DRM_MODE_FLAG_DBLCLK |		\
>  				 DRM_MODE_FLAG_CLKDIV2 |	\
> +				 DRM_MODE_FLAG_LOW_POWER |	\
>  				 DRM_MODE_FLAG_3D_MASK)
>  
>  /* DPMS flags */

Hey,

The wording seems to be a bit generic, with just the description I have no idea how to implement this in userspace, do you have an implementation as well?

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm: add client cap to expose low power modes
  2020-10-21  6:54 [PATCH] drm: add client cap to expose low power modes Ken Huang
  2020-10-21  7:12 ` Maarten Lankhorst
@ 2020-10-21  7:40 ` Simon Ser
  2020-10-21  8:34   ` Daniel Vetter
  1 sibling, 1 reply; 10+ messages in thread
From: Simon Ser @ 2020-10-21  7:40 UTC (permalink / raw)
  To: Ken Huang; +Cc: Thomas Zimmermann, Adrian Salido, dri-devel

On Wednesday, October 21, 2020 8:54 AM, Ken Huang <kenbshuang@google.com> wrote:

> From: Adrian Salido salidoa@google.com
>
> Some displays may support Low Power modes, however, these modes may
> require special handling as they would usually require lower OPR
> content on framebuffers.

I'm not familiar with OPR. Can you explain what it is and what it means
for user-space?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm: add client cap to expose low power modes
  2020-10-21  7:40 ` Simon Ser
@ 2020-10-21  8:34   ` Daniel Vetter
  2020-10-21 14:58     ` Ken Huang
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2020-10-21  8:34 UTC (permalink / raw)
  To: Simon Ser; +Cc: Thomas Zimmermann, Adrian Salido, dri-devel, Ken Huang

On Wed, Oct 21, 2020 at 07:40:48AM +0000, Simon Ser wrote:
> On Wednesday, October 21, 2020 8:54 AM, Ken Huang <kenbshuang@google.com> wrote:
> 
> > From: Adrian Salido salidoa@google.com
> >
> > Some displays may support Low Power modes, however, these modes may
> > require special handling as they would usually require lower OPR
> > content on framebuffers.
> 
> I'm not familiar with OPR. Can you explain what it is and what it means
> for user-space?

Also since this is new uapi, I guess best explanation would include the
userspace code that makes sensible use of this.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm: add client cap to expose low power modes
  2020-10-21  8:34   ` Daniel Vetter
@ 2020-10-21 14:58     ` Ken Huang
  2020-10-21 15:58       ` Daniel Vetter
  0 siblings, 1 reply; 10+ messages in thread
From: Ken Huang @ 2020-10-21 14:58 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Adrian Salido, dri-devel, Thomas Zimmermann


[-- Attachment #1.1: Type: text/plain, Size: 1683 bytes --]

Hi All,

It's useful in Android and other embedded devices to implement Always On
Display (ex. showing clock faces with less than 15% OPR on screen).

OPR (On Pixel Ratio) is the percentage of luminance amount over the display
area.
It's derived by gray levels of display image pattern and the backlight (or
OLED) driving force (or current).
ex: OPR=100% means a full white pattern with maximum backlight (or OLED)
brightness, while full black would be OPR=0%.

In userspace, when the client initializes, we can set capability via
drmSetClientCap() to ask the display driver to expose the drm modes with
DRM_MODE_FLAG_LOW_POWER flag.
Userspace can check DRM_MODE_FLAG_LOW_POWER flag to know which modes can be
used to consume the least amount of power during Always On Display.
Ignoring modes with this flag set during normal operating mode.

Thanks,
Ken

Daniel Vetter <daniel@ffwll.ch> 於 2020年10月21日 週三 下午4:34寫道:

> On Wed, Oct 21, 2020 at 07:40:48AM +0000, Simon Ser wrote:
> > On Wednesday, October 21, 2020 8:54 AM, Ken Huang <kenbshuang@google.com>
> wrote:
> >
> > > From: Adrian Salido salidoa@google.com
> > >
> > > Some displays may support Low Power modes, however, these modes may
> > > require special handling as they would usually require lower OPR
> > > content on framebuffers.
> >
> > I'm not familiar with OPR. Can you explain what it is and what it means
> > for user-space?
>
> Also since this is new uapi, I guess best explanation would include the
> userspace code that makes sensible use of this.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>

[-- Attachment #1.2: Type: text/html, Size: 2343 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm: add client cap to expose low power modes
  2020-10-21 14:58     ` Ken Huang
@ 2020-10-21 15:58       ` Daniel Vetter
  2020-10-21 16:11         ` Daniel Stone
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2020-10-21 15:58 UTC (permalink / raw)
  To: Ken Huang; +Cc: Adrian Salido, dri-devel, Thomas Zimmermann

On Wed, Oct 21, 2020 at 4:59 PM Ken Huang <kenbshuang@google.com> wrote:
>
> Hi All,
>
> It's useful in Android and other embedded devices to implement Always On Display (ex. showing clock faces with less than 15% OPR on screen).
>
> OPR (On Pixel Ratio) is the percentage of luminance amount over the display area.
> It's derived by gray levels of display image pattern and the backlight (or OLED) driving force (or current).
> ex: OPR=100% means a full white pattern with maximum backlight (or OLED) brightness, while full black would be OPR=0%.
>
> In userspace, when the client initializes, we can set capability via drmSetClientCap() to ask the display driver to expose the drm modes with DRM_MODE_FLAG_LOW_POWER flag.
> Userspace can check DRM_MODE_FLAG_LOW_POWER flag to know which modes can be used to consume the least amount of power during Always On Display.
> Ignoring modes with this flag set during normal operating mode.

Hm I'm not really sure what this changes ... I think we need the
entire pile of patches: Userspace, driver, drm core, anything else.
Just adding this flag doesn't make much sense really, since I have no
idea what the semantics are. Even after you've explained the use-case.

Also for new kms uapi we need an igt testcase to exercise this and
make sure it works.
-Daniel

>
> Thanks,
> Ken
>
> Daniel Vetter <daniel@ffwll.ch> 於 2020年10月21日 週三 下午4:34寫道:
>>
>> On Wed, Oct 21, 2020 at 07:40:48AM +0000, Simon Ser wrote:
>> > On Wednesday, October 21, 2020 8:54 AM, Ken Huang <kenbshuang@google.com> wrote:
>> >
>> > > From: Adrian Salido salidoa@google.com
>> > >
>> > > Some displays may support Low Power modes, however, these modes may
>> > > require special handling as they would usually require lower OPR
>> > > content on framebuffers.
>> >
>> > I'm not familiar with OPR. Can you explain what it is and what it means
>> > for user-space?
>>
>> Also since this is new uapi, I guess best explanation would include the
>> userspace code that makes sensible use of this.
>> -Daniel
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm: add client cap to expose low power modes
  2020-10-21 15:58       ` Daniel Vetter
@ 2020-10-21 16:11         ` Daniel Stone
  2020-10-21 16:34           ` Daniel Vetter
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Stone @ 2020-10-21 16:11 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Ken Huang, Thomas Zimmermann, dri-devel, Adrian Salido

On Wed, 21 Oct 2020 at 16:58, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Oct 21, 2020 at 4:59 PM Ken Huang <kenbshuang@google.com> wrote:
> > It's useful in Android and other embedded devices to implement Always On Display (ex. showing clock faces with less than 15% OPR on screen).
> >
> > OPR (On Pixel Ratio) is the percentage of luminance amount over the display area.
> > It's derived by gray levels of display image pattern and the backlight (or OLED) driving force (or current).
> > ex: OPR=100% means a full white pattern with maximum backlight (or OLED) brightness, while full black would be OPR=0%.
> >
> > In userspace, when the client initializes, we can set capability via drmSetClientCap() to ask the display driver to expose the drm modes with DRM_MODE_FLAG_LOW_POWER flag.
> > Userspace can check DRM_MODE_FLAG_LOW_POWER flag to know which modes can be used to consume the least amount of power during Always On Display.
> > Ignoring modes with this flag set during normal operating mode.
>
> Hm I'm not really sure what this changes ... I think we need the
> entire pile of patches: Userspace, driver, drm core, anything else.
> Just adding this flag doesn't make much sense really, since I have no
> idea what the semantics are. Even after you've explained the use-case.

It makes sense to me: some modes are annotated with a 'low-power'
flag, tucked away behind a client cap which makes clients opt in, and
they can switch into the low-power mode (letting the display/panel
save a lot of power) _if_ they only have at most 15% of pixels lit up.

My worry is about the 15% though ... what happens when hardware allows
up to 20%, or only allows 10%?

If we can reuse the same modelines, then rather than create new
modelines just for low-power modes, I'd rather create a client CRTC
property specifying the number/percentages of pixels on the CRTC which
are lit non-zero. That would give us more wriggle room to change the
semantics, as well as redefine 'low power' in terms of
monochrome/scaled/non-bright/etc modes. But it does make the
switching-between-clients problem even worse than it already is.

Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm: add client cap to expose low power modes
  2020-10-21 16:11         ` Daniel Stone
@ 2020-10-21 16:34           ` Daniel Vetter
  2020-10-21 17:09             ` Daniel Stone
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2020-10-21 16:34 UTC (permalink / raw)
  To: Daniel Stone; +Cc: Thomas Zimmermann, Ken Huang, dri-devel, Adrian Salido

On Wed, Oct 21, 2020 at 05:11:00PM +0100, Daniel Stone wrote:
> On Wed, 21 Oct 2020 at 16:58, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Wed, Oct 21, 2020 at 4:59 PM Ken Huang <kenbshuang@google.com> wrote:
> > > It's useful in Android and other embedded devices to implement Always On Display (ex. showing clock faces with less than 15% OPR on screen).
> > >
> > > OPR (On Pixel Ratio) is the percentage of luminance amount over the display area.
> > > It's derived by gray levels of display image pattern and the backlight (or OLED) driving force (or current).
> > > ex: OPR=100% means a full white pattern with maximum backlight (or OLED) brightness, while full black would be OPR=0%.
> > >
> > > In userspace, when the client initializes, we can set capability via drmSetClientCap() to ask the display driver to expose the drm modes with DRM_MODE_FLAG_LOW_POWER flag.
> > > Userspace can check DRM_MODE_FLAG_LOW_POWER flag to know which modes can be used to consume the least amount of power during Always On Display.
> > > Ignoring modes with this flag set during normal operating mode.
> >
> > Hm I'm not really sure what this changes ... I think we need the
> > entire pile of patches: Userspace, driver, drm core, anything else.
> > Just adding this flag doesn't make much sense really, since I have no
> > idea what the semantics are. Even after you've explained the use-case.
> 
> It makes sense to me: some modes are annotated with a 'low-power'
> flag, tucked away behind a client cap which makes clients opt in, and
> they can switch into the low-power mode (letting the display/panel
> save a lot of power) _if_ they only have at most 15% of pixels lit up.
> 
> My worry is about the 15% though ... what happens when hardware allows
> up to 20%, or only allows 10%?

Yeah exactly, that's what I'm worried about too, these kind of details.
Opt-in flag for special modes, no problem, but we need to make sure we
agree on what flavour of special exactly they are.

> If we can reuse the same modelines, then rather than create new
> modelines just for low-power modes, I'd rather create a client CRTC
> property specifying the number/percentages of pixels on the CRTC which
> are lit non-zero. That would give us more wriggle room to change the
> semantics, as well as redefine 'low power' in terms of
> monochrome/scaled/non-bright/etc modes. But it does make the
> switching-between-clients problem even worse than it already is.

Yeah, that would make sense too. Or maybe even add read-only hint that
says "if you're below 15% non-black we can do low power for your, please
be nice".
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm: add client cap to expose low power modes
  2020-10-21 16:34           ` Daniel Vetter
@ 2020-10-21 17:09             ` Daniel Stone
  2020-10-22  7:36               ` Pekka Paalanen
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Stone @ 2020-10-21 17:09 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Ken Huang, Thomas Zimmermann, dri-devel, Adrian Salido

On Wed, 21 Oct 2020 at 17:34, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Oct 21, 2020 at 05:11:00PM +0100, Daniel Stone wrote:
> > It makes sense to me: some modes are annotated with a 'low-power'
> > flag, tucked away behind a client cap which makes clients opt in, and
> > they can switch into the low-power mode (letting the display/panel
> > save a lot of power) _if_ they only have at most 15% of pixels lit up.
> >
> > My worry is about the 15% though ... what happens when hardware allows
> > up to 20%, or only allows 10%?
>
> Yeah exactly, that's what I'm worried about too, these kind of details.
> Opt-in flag for special modes, no problem, but we need to make sure we
> agree on what flavour of special exactly they are.
>
> > If we can reuse the same modelines, then rather than create new
> > modelines just for low-power modes, I'd rather create a client CRTC
> > property specifying the number/percentages of pixels on the CRTC which
> > are lit non-zero. That would give us more wriggle room to change the
> > semantics, as well as redefine 'low power' in terms of
> > monochrome/scaled/non-bright/etc modes. But it does make the
> > switching-between-clients problem even worse than it already is.
>
> Yeah, that would make sense too. Or maybe even add read-only hint that
> says "if you're below 15% non-black we can do low power for your, please
> be nice".

If the hardware can actually do that autonomously then great, but I'm
guessing the reason we're talking about separate opt-in modes here is
that it can't.

Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm: add client cap to expose low power modes
  2020-10-21 17:09             ` Daniel Stone
@ 2020-10-22  7:36               ` Pekka Paalanen
  0 siblings, 0 replies; 10+ messages in thread
From: Pekka Paalanen @ 2020-10-22  7:36 UTC (permalink / raw)
  To: Ken Huang; +Cc: Thomas Zimmermann, dri-devel, Adrian Salido


[-- Attachment #1.1: Type: text/plain, Size: 4049 bytes --]

On Wed, 21 Oct 2020 18:09:05 +0100
Daniel Stone <daniel@fooishbar.org> wrote:

> On Wed, 21 Oct 2020 at 17:34, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Wed, Oct 21, 2020 at 05:11:00PM +0100, Daniel Stone wrote:  
> > > It makes sense to me: some modes are annotated with a 'low-power'
> > > flag, tucked away behind a client cap which makes clients opt in, and
> > > they can switch into the low-power mode (letting the display/panel
> > > save a lot of power) _if_ they only have at most 15% of pixels lit up.
> > >
> > > My worry is about the 15% though ... what happens when hardware allows
> > > up to 20%, or only allows 10%?  
> >
> > Yeah exactly, that's what I'm worried about too, these kind of details.
> > Opt-in flag for special modes, no problem, but we need to make sure we
> > agree on what flavour of special exactly they are.

Hi,

I second those concerns.

I would also like to hear the reason why low power should be tied to a
video mode instead of being an orthogonal property. Does low power
depend on different timings? Different resolution?

Maybe extremely low refresh rate plays a role here? Or maybe it goes
into panel self-refresh mode that is somehow different from normal
timing wise?

How does low power mode interact with backlight controls? Does low
power mode automatically change the backlight control range, or the
control value, or does userspace need to dim down the backlight
explicitly, or what should happen?

What if userspace does not do what the driver expects? E.g. the
framebuffer contains too much too bright pixels, or the backlight
control is not set appropriately? What may happen on screen in those
cases?

> > > If we can reuse the same modelines, then rather than create new
> > > modelines just for low-power modes, I'd rather create a client CRTC
> > > property specifying the number/percentages of pixels on the CRTC which
> > > are lit non-zero. That would give us more wriggle room to change the
> > > semantics, as well as redefine 'low power' in terms of
> > > monochrome/scaled/non-bright/etc modes. But it does make the
> > > switching-between-clients problem even worse than it already is.  

That would seem better to me too, but I got the impression that rather
than non-zero, many dim pixels would be ok in low-power too. So that
may require specifying the formula for how exactly to calculate the
percentage. Mind, that power consumption need not be linear with
luminance, so if power consumption is the primary factor then writing
it down as luminance may not be correct.

Of course, the calculation should be simple and conservative enough
that it can be used with many kinds of hardware.

Also, HDR monitors may have a similar issue: a monitor may be limited
to certain wattage, which means that either you can display a small and
very bright patch, or a large and not that bright patch. It's unclear
to me if the same mechanism would be appropriate for both limiting HDR
display power under normal conditions and cell-phone display power in
low-power conditions.

Maybe "low power" should not even be a yes/no property, but a value
range, like wattage?

I think the problem with switching between KMS clients is something we
just have to solve by userspace restoring also unrecognized KMS
properties on switch-in always, like I have written before. I see no
way around it given the policy that the kernel will not offer any kind
of "defaults" property set (which too would need to be explicitly
used by userspace, or maybe a cap to stop the kernel from applying
it automatically whenever something gains DRM master).


Thanks,
pq

> > Yeah, that would make sense too. Or maybe even add read-only hint that
> > says "if you're below 15% non-black we can do low power for your, please
> > be nice".  
> 
> If the hardware can actually do that autonomously then great, but I'm
> guessing the reason we're talking about separate opt-in modes here is
> that it can't.
> 
> Cheers,
> Daniel

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-10-22  7:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-21  6:54 [PATCH] drm: add client cap to expose low power modes Ken Huang
2020-10-21  7:12 ` Maarten Lankhorst
2020-10-21  7:40 ` Simon Ser
2020-10-21  8:34   ` Daniel Vetter
2020-10-21 14:58     ` Ken Huang
2020-10-21 15:58       ` Daniel Vetter
2020-10-21 16:11         ` Daniel Stone
2020-10-21 16:34           ` Daniel Vetter
2020-10-21 17:09             ` Daniel Stone
2020-10-22  7:36               ` Pekka Paalanen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.