Re: [PATCH] drm: add client cap to expose low power modes
On Wed, 21 Oct 2020 18:09:05 +0100 Daniel Stone wrote: > On Wed, 21 Oct 2020 at 17:34, Daniel Vetter 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 pgpeDpSoa7NbU.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: add client cap to expose low power modes
On Wed, 21 Oct 2020 at 17:34, Daniel Vetter 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
Re: [PATCH] drm: add client cap to expose low power modes
On Wed, Oct 21, 2020 at 05:11:00PM +0100, Daniel Stone wrote: > On Wed, 21 Oct 2020 at 16:58, Daniel Vetter wrote: > > On Wed, Oct 21, 2020 at 4:59 PM Ken Huang 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
Re: [PATCH] drm: add client cap to expose low power modes
On Wed, 21 Oct 2020 at 16:58, Daniel Vetter wrote: > On Wed, Oct 21, 2020 at 4:59 PM Ken Huang 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
Re: [PATCH] drm: add client cap to expose low power modes
On Wed, Oct 21, 2020 at 4:59 PM Ken Huang 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 於 2020年10月21日 週三 下午4:34寫道： >> >> On Wed, Oct 21, 2020 at 07:40:48AM +, Simon Ser wrote: >> > On Wednesday, October 21, 2020 8:54 AM, Ken Huang >> > wrote: >> > >> > > From: Adrian Salido sali...@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
Re: [PATCH] drm: add client cap to expose low power modes
On Wed, Oct 21, 2020 at 07:40:48AM +, Simon Ser wrote: > On Wednesday, October 21, 2020 8:54 AM, Ken Huang > wrote: > > > From: Adrian Salido sali...@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
Re: [PATCH] drm: add client cap to expose low power modes
On Wednesday, October 21, 2020 8:54 AM, Ken Huang wrote: > From: Adrian Salido sali...@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
Re: [PATCH] drm: add client cap to expose low power modes
Op 21-10-2020 om 08:54 schreef Ken Huang:
> From: Adrian Salido
>
> 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
> Signed-off-by: Ken Huang
> ---
> 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
