[RFC] Expanding drm_mode_modeinfo flags

[RFC] Expanding drm_mode_modeinfo flags

[Jeykumar Sankaran]

    Hello All, 
    	drm_mode_modeinfo::flags is a 32 bit field currently used to
    describe the properties of a connector mode. I see the least order 22 bits
    are already in use. Posting this RFC to discuss on any potential plans to 
    expand the bit range support of this field for growing mode properties and 
    ways to handle one such property needed by the msm dpu driver.

    msm drivers support panels which can dynamically switch between
    video(active) and command(smart) modes. Within video mode, they also support
    switching between resolutions seamlessly i.e. glitch free resolution switch.
    But they cannot do a seamless switch from a resolutions from video to
    command or vice versa. Clients need to be aware for these capablities before
    they switch between the resolutions. Since these capabilities are identified
    per drm_mode, we are considering the below two approaches to handle this
    use case.

    Option 1:
    Attached patch adds flag values to associate a drm_mode to video/command
    mode and to indicate its capability to do a seamless switch.

    Option 2:
    drm_mode_modeinfo can expose a new "private_flags" field to handle vendor
    specific mode flags. Besides the above mentioned use case, we are also
    expoloring methods to handle some of our display port resolution switch use
    cases where the DP ports can be operated in a tiled/detiled modes. This 
    approach will provide a standard channel for drm driver vendors for their 
    growing need for drm_mode specific capabilities.

    Please provide your inputs on the options or any upstream friendly
    recommendation to handle such custom use cases.

    Thanks and Regards,
    Jeykumar S.

Jeykumar Sankaran (1):
  drm: add mode flags in uapi for seamless mode switch

 include/uapi/drm/drm_mode.h | 5 +++++
 1 file changed, 5 insertions(+)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[RFC] Expanding drm_mode_modeinfo flags

[Jeykumar Sankaran]

    Hello All, 
    	drm_mode_modeinfo::flags is a 32 bit field currently used to
    describe the properties of a connector mode. I see the least order 22 bits
    are already in use. Posting this RFC to discuss on any potential plans to 
    expand the bit range support of this field for growing mode properties and 
    ways to handle one such property needed by the msm dpu driver.

    msm drivers support panels which can dynamically switch between
    video(active) and command(smart) modes. Within video mode, they also support
    switching between resolutions seamlessly i.e. glitch free resolution switch.
    But they cannot do a seamless switch from a resolutions from video to
    command or vice versa. Clients need to be aware for these capablities before
    they switch between the resolutions. Since these capabilities are identified
    per drm_mode, we are considering the below two approaches to handle this
    use case.

    Option 1:
    Attached patch adds flag values to associate a drm_mode to video/command
    mode and to indicate its capability to do a seamless switch.

    Option 2:
    drm_mode_modeinfo can expose a new "private_flags" field to handle vendor
    specific mode flags. Besides the above mentioned use case, we are also
    expoloring methods to handle some of our display port resolution switch use
    cases where the DP ports can be operated in a tiled/detiled modes. This 
    approach will provide a standard channel for drm driver vendors for their 
    growing need for drm_mode specific capabilities.

    Please provide your inputs on the options or any upstream friendly
    recommendation to handle such custom use cases.

    Thanks and Regards,
    Jeykumar S.

Jeykumar Sankaran (1):
  drm: add mode flags in uapi for seamless mode switch

 include/uapi/drm/drm_mode.h | 5 +++++
 1 file changed, 5 insertions(+)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[RFC PATCH] drm: add mode flags in uapi for seamless mode switch

[Jeykumar Sankaran]

Add drm mode flag values to expose mode capabilities to
perform dynamic seamless mode switch. This change also
exposes the backing panel type associated with a mode
for panels which can dynamically switch between video
and command display modes.

Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 include/uapi/drm/drm_mode.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 5ab331e..b76f1bf 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -124,6 +124,11 @@
 #define  DRM_MODE_FLAG_PIC_AR_256_135 \
 			(DRM_MODE_PICTURE_ASPECT_256_135<<19)
 
+#define DRM_MODE_FLAG_SEAMLESS_SWITCH (1<<23)
+
+#define DRM_MODE_FLAG_VIDEO_MODE   (1<<24)
+#define DRM_MODE_FLAG_COMMAND_MODE (1<<25)
+
 #define  DRM_MODE_FLAG_ALL	(DRM_MODE_FLAG_PHSYNC |		\
 				 DRM_MODE_FLAG_NHSYNC |		\
 				 DRM_MODE_FLAG_PVSYNC |		\
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[RFC PATCH] drm: add mode flags in uapi for seamless mode switch

[Jeykumar Sankaran]

Add drm mode flag values to expose mode capabilities to
perform dynamic seamless mode switch. This change also
exposes the backing panel type associated with a mode
for panels which can dynamically switch between video
and command display modes.

Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 include/uapi/drm/drm_mode.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 5ab331e..b76f1bf 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -124,6 +124,11 @@
 #define  DRM_MODE_FLAG_PIC_AR_256_135 \
 			(DRM_MODE_PICTURE_ASPECT_256_135<<19)
 
+#define DRM_MODE_FLAG_SEAMLESS_SWITCH (1<<23)
+
+#define DRM_MODE_FLAG_VIDEO_MODE   (1<<24)
+#define DRM_MODE_FLAG_COMMAND_MODE (1<<25)
+
 #define  DRM_MODE_FLAG_ALL	(DRM_MODE_FLAG_PHSYNC |		\
 				 DRM_MODE_FLAG_NHSYNC |		\
 				 DRM_MODE_FLAG_PVSYNC |		\
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

Re: [RFC] Expanding drm_mode_modeinfo flags

[Daniel Vetter]

On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:
>     Hello All, 
>     	drm_mode_modeinfo::flags is a 32 bit field currently used to
>     describe the properties of a connector mode. I see the least order 22 bits
>     are already in use. Posting this RFC to discuss on any potential plans to 
>     expand the bit range support of this field for growing mode properties and 
>     ways to handle one such property needed by the msm dpu driver.
> 
>     msm drivers support panels which can dynamically switch between
>     video(active) and command(smart) modes. Within video mode, they also support
>     switching between resolutions seamlessly i.e. glitch free resolution switch.
>     But they cannot do a seamless switch from a resolutions from video to
>     command or vice versa. Clients need to be aware for these capablities before
>     they switch between the resolutions. Since these capabilities are identified
>     per drm_mode, we are considering the below two approaches to handle this
>     use case.
> 
>     Option 1:
>     Attached patch adds flag values to associate a drm_mode to video/command
>     mode and to indicate its capability to do a seamless switch.
> 
>     Option 2:
>     drm_mode_modeinfo can expose a new "private_flags" field to handle vendor
>     specific mode flags. Besides the above mentioned use case, we are also
>     expoloring methods to handle some of our display port resolution switch use
>     cases where the DP ports can be operated in a tiled/detiled modes. This 
>     approach will provide a standard channel for drm driver vendors for their 
>     growing need for drm_mode specific capabilities.
> 
>     Please provide your inputs on the options or any upstream friendly
>     recommendation to handle such custom use cases.
> 
>     Thanks and Regards,
>     Jeykumar S.
> 
> Jeykumar Sankaran (1):
>   drm: add mode flags in uapi for seamless mode switch

I think the uapi is the trivial part here, the real deal is how userspace
uses this. Can you pls post the patches for your compositor?

Also note that we already allow userspace to tell the kernel whether
flickering is ok or not for a modeset. msm driver could use that to at
least tell userspace whether a modeset change is possible. So you can
already implement glitch-free modeset changes for at least video mode.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [RFC] Expanding drm_mode_modeinfo flags

[Daniel Vetter]

On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:
>     Hello All, 
>     	drm_mode_modeinfo::flags is a 32 bit field currently used to
>     describe the properties of a connector mode. I see the least order 22 bits
>     are already in use. Posting this RFC to discuss on any potential plans to 
>     expand the bit range support of this field for growing mode properties and 
>     ways to handle one such property needed by the msm dpu driver.
> 
>     msm drivers support panels which can dynamically switch between
>     video(active) and command(smart) modes. Within video mode, they also support
>     switching between resolutions seamlessly i.e. glitch free resolution switch.
>     But they cannot do a seamless switch from a resolutions from video to
>     command or vice versa. Clients need to be aware for these capablities before
>     they switch between the resolutions. Since these capabilities are identified
>     per drm_mode, we are considering the below two approaches to handle this
>     use case.
> 
>     Option 1:
>     Attached patch adds flag values to associate a drm_mode to video/command
>     mode and to indicate its capability to do a seamless switch.
> 
>     Option 2:
>     drm_mode_modeinfo can expose a new "private_flags" field to handle vendor
>     specific mode flags. Besides the above mentioned use case, we are also
>     expoloring methods to handle some of our display port resolution switch use
>     cases where the DP ports can be operated in a tiled/detiled modes. This 
>     approach will provide a standard channel for drm driver vendors for their 
>     growing need for drm_mode specific capabilities.
> 
>     Please provide your inputs on the options or any upstream friendly
>     recommendation to handle such custom use cases.
> 
>     Thanks and Regards,
>     Jeykumar S.
> 
> Jeykumar Sankaran (1):
>   drm: add mode flags in uapi for seamless mode switch

I think the uapi is the trivial part here, the real deal is how userspace
uses this. Can you pls post the patches for your compositor?

Also note that we already allow userspace to tell the kernel whether
flickering is ok or not for a modeset. msm driver could use that to at
least tell userspace whether a modeset change is possible. So you can
already implement glitch-free modeset changes for at least video mode.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [RFC] Expanding drm_mode_modeinfo flags

[Jeykumar Sankaran]

On 2019-07-16 02:07, Daniel Vetter wrote:
> On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:
>>     Hello All,
>>     	drm_mode_modeinfo::flags is a 32 bit field currently used to
>>     describe the properties of a connector mode. I see the least order
> 22 bits
>>     are already in use. Posting this RFC to discuss on any potential
> plans to
>>     expand the bit range support of this field for growing mode
> properties and
>>     ways to handle one such property needed by the msm dpu driver.
>> 
>>     msm drivers support panels which can dynamically switch between
>>     video(active) and command(smart) modes. Within video mode, they 
>> also
> support
>>     switching between resolutions seamlessly i.e. glitch free 
>> resolution
> switch.
>>     But they cannot do a seamless switch from a resolutions from video
> to
>>     command or vice versa. Clients need to be aware for these
> capablities before
>>     they switch between the resolutions. Since these capabilities are
> identified
>>     per drm_mode, we are considering the below two approaches to 
>> handle
> this
>>     use case.
>> 
>>     Option 1:
>>     Attached patch adds flag values to associate a drm_mode to
> video/command
>>     mode and to indicate its capability to do a seamless switch.
>> 
>>     Option 2:
>>     drm_mode_modeinfo can expose a new "private_flags" field to handle
> vendor
>>     specific mode flags. Besides the above mentioned use case, we are
> also
>>     expoloring methods to handle some of our display port resolution
> switch use
>>     cases where the DP ports can be operated in a tiled/detiled modes.
> This
>>     approach will provide a standard channel for drm driver vendors 
>> for
> their
>>     growing need for drm_mode specific capabilities.
>> 
>>     Please provide your inputs on the options or any upstream friendly
>>     recommendation to handle such custom use cases.
>> 
>>     Thanks and Regards,
>>     Jeykumar S.
>> 
>> Jeykumar Sankaran (1):
>>   drm: add mode flags in uapi for seamless mode switch
> 
> I think the uapi is the trivial part here, the real deal is how 
> userspace
> uses this. Can you pls post the patches for your compositor?
> 
> Also note that we already allow userspace to tell the kernel whether
> flickering is ok or not for a modeset. msm driver could use that to at
> least tell userspace whether a modeset change is possible. So you can
> already implement glitch-free modeset changes for at least video mode.
> -Daniel

I believe you are referring to the below tv property of the connector.

/**
  * @tv_flicker_reduction_property: Optional TV property to control the
  * flicker reduction mode.
  */
struct drm_property *tv_flicker_reduction_property;

Sure. We can use this to indicate whether the connector representing the 
panel
can support the dynamic glitch-free switch. But when the panel supports 
both
video and command mode resolutions and such glitch-free switch is 
possible only between
video mode resolutions, we need per resolution(drm_mode_modeinfo) 
information
to identify the resolutions enumerated for video mode.

Below is an example of the compositor utility function which can use the 
tv_flicker_property
and the proposed modeinfo flags to identify glitch-free switches.

  bool is_seamless_switch_supported(struct drm_mode_modeinfo src_mode, 
struct
                  drm_mode_modeinfo *dst_mode, bool 
flicker_reduction_supported)
  {
          if (!flicker_reduction_supported) {
                  printf("flicker reduction prop not set for the 
connector\n");
                  return false;
          }

          /*
           * Seamless switch(if supported) is possible only between video 
mode
           * resolutions
           */
          if (src_mode->flags & DRM_MODE_FLAG_VIDEO_MODE &&
                          dst_mode->flags & DRM_MODE_FLAG_VIDEO_MODE)
                  return true;
          else
                  return false;

  }


-- 
Jeykumar S

Re: [RFC] Expanding drm_mode_modeinfo flags

[Jeykumar Sankaran]

On 2019-07-16 02:07, Daniel Vetter wrote:
> On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:
>>     Hello All,
>>     	drm_mode_modeinfo::flags is a 32 bit field currently used to
>>     describe the properties of a connector mode. I see the least order
> 22 bits
>>     are already in use. Posting this RFC to discuss on any potential
> plans to
>>     expand the bit range support of this field for growing mode
> properties and
>>     ways to handle one such property needed by the msm dpu driver.
>> 
>>     msm drivers support panels which can dynamically switch between
>>     video(active) and command(smart) modes. Within video mode, they 
>> also
> support
>>     switching between resolutions seamlessly i.e. glitch free 
>> resolution
> switch.
>>     But they cannot do a seamless switch from a resolutions from video
> to
>>     command or vice versa. Clients need to be aware for these
> capablities before
>>     they switch between the resolutions. Since these capabilities are
> identified
>>     per drm_mode, we are considering the below two approaches to 
>> handle
> this
>>     use case.
>> 
>>     Option 1:
>>     Attached patch adds flag values to associate a drm_mode to
> video/command
>>     mode and to indicate its capability to do a seamless switch.
>> 
>>     Option 2:
>>     drm_mode_modeinfo can expose a new "private_flags" field to handle
> vendor
>>     specific mode flags. Besides the above mentioned use case, we are
> also
>>     expoloring methods to handle some of our display port resolution
> switch use
>>     cases where the DP ports can be operated in a tiled/detiled modes.
> This
>>     approach will provide a standard channel for drm driver vendors 
>> for
> their
>>     growing need for drm_mode specific capabilities.
>> 
>>     Please provide your inputs on the options or any upstream friendly
>>     recommendation to handle such custom use cases.
>> 
>>     Thanks and Regards,
>>     Jeykumar S.
>> 
>> Jeykumar Sankaran (1):
>>   drm: add mode flags in uapi for seamless mode switch
> 
> I think the uapi is the trivial part here, the real deal is how 
> userspace
> uses this. Can you pls post the patches for your compositor?
> 
> Also note that we already allow userspace to tell the kernel whether
> flickering is ok or not for a modeset. msm driver could use that to at
> least tell userspace whether a modeset change is possible. So you can
> already implement glitch-free modeset changes for at least video mode.
> -Daniel

I believe you are referring to the below tv property of the connector.

/**
  * @tv_flicker_reduction_property: Optional TV property to control the
  * flicker reduction mode.
  */
struct drm_property *tv_flicker_reduction_property;

Sure. We can use this to indicate whether the connector representing the 
panel
can support the dynamic glitch-free switch. But when the panel supports 
both
video and command mode resolutions and such glitch-free switch is 
possible only between
video mode resolutions, we need per resolution(drm_mode_modeinfo) 
information
to identify the resolutions enumerated for video mode.

Below is an example of the compositor utility function which can use the 
tv_flicker_property
and the proposed modeinfo flags to identify glitch-free switches.

  bool is_seamless_switch_supported(struct drm_mode_modeinfo src_mode, 
struct
                  drm_mode_modeinfo *dst_mode, bool 
flicker_reduction_supported)
  {
          if (!flicker_reduction_supported) {
                  printf("flicker reduction prop not set for the 
connector\n");
                  return false;
          }

          /*
           * Seamless switch(if supported) is possible only between video 
mode
           * resolutions
           */
          if (src_mode->flags & DRM_MODE_FLAG_VIDEO_MODE &&
                          dst_mode->flags & DRM_MODE_FLAG_VIDEO_MODE)
                  return true;
          else
                  return false;

  }


-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [RFC] Expanding drm_mode_modeinfo flags

[Daniel Vetter]

On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran wrote:
> On 2019-07-16 02:07, Daniel Vetter wrote:
> > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:
> > >     Hello All,
> > >     	drm_mode_modeinfo::flags is a 32 bit field currently used to
> > >     describe the properties of a connector mode. I see the least order
> > 22 bits
> > >     are already in use. Posting this RFC to discuss on any potential
> > plans to
> > >     expand the bit range support of this field for growing mode
> > properties and
> > >     ways to handle one such property needed by the msm dpu driver.
> > > 
> > >     msm drivers support panels which can dynamically switch between
> > >     video(active) and command(smart) modes. Within video mode, they
> > > also
> > support
> > >     switching between resolutions seamlessly i.e. glitch free
> > > resolution
> > switch.
> > >     But they cannot do a seamless switch from a resolutions from video
> > to
> > >     command or vice versa. Clients need to be aware for these
> > capablities before
> > >     they switch between the resolutions. Since these capabilities are
> > identified
> > >     per drm_mode, we are considering the below two approaches to
> > > handle
> > this
> > >     use case.
> > > 
> > >     Option 1:
> > >     Attached patch adds flag values to associate a drm_mode to
> > video/command
> > >     mode and to indicate its capability to do a seamless switch.
> > > 
> > >     Option 2:
> > >     drm_mode_modeinfo can expose a new "private_flags" field to handle
> > vendor
> > >     specific mode flags. Besides the above mentioned use case, we are
> > also
> > >     expoloring methods to handle some of our display port resolution
> > switch use
> > >     cases where the DP ports can be operated in a tiled/detiled modes.
> > This
> > >     approach will provide a standard channel for drm driver vendors
> > > for
> > their
> > >     growing need for drm_mode specific capabilities.
> > > 
> > >     Please provide your inputs on the options or any upstream friendly
> > >     recommendation to handle such custom use cases.
> > > 
> > >     Thanks and Regards,
> > >     Jeykumar S.
> > > 
> > > Jeykumar Sankaran (1):
> > >   drm: add mode flags in uapi for seamless mode switch
> > 
> > I think the uapi is the trivial part here, the real deal is how
> > userspace
> > uses this. Can you pls post the patches for your compositor?
> > 
> > Also note that we already allow userspace to tell the kernel whether
> > flickering is ok or not for a modeset. msm driver could use that to at
> > least tell userspace whether a modeset change is possible. So you can
> > already implement glitch-free modeset changes for at least video mode.
> > -Daniel
> 
> I believe you are referring to the below tv property of the connector.
> 
> /**
>  * @tv_flicker_reduction_property: Optional TV property to control the
>  * flicker reduction mode.
>  */
> struct drm_property *tv_flicker_reduction_property;

Not even close :-)

I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic ioctl. This
is not a property of a mode, this is a property of a _transition_ between
configurations. Some transitions can be done flicker free, others can't.

There's then still the question of how to pick video vs command mode, but
imo better to start with implementing the transition behaviour correctly
first.
-Daniel

> 
> Sure. We can use this to indicate whether the connector representing the
> panel
> can support the dynamic glitch-free switch. But when the panel supports both
> video and command mode resolutions and such glitch-free switch is possible
> only between
> video mode resolutions, we need per resolution(drm_mode_modeinfo)
> information
> to identify the resolutions enumerated for video mode.
> 
> Below is an example of the compositor utility function which can use the
> tv_flicker_property
> and the proposed modeinfo flags to identify glitch-free switches.
> 
>  bool is_seamless_switch_supported(struct drm_mode_modeinfo src_mode, struct
>                  drm_mode_modeinfo *dst_mode, bool
> flicker_reduction_supported)
>  {
>          if (!flicker_reduction_supported) {
>                  printf("flicker reduction prop not set for the
> connector\n");
>                  return false;
>          }
> 
>          /*
>           * Seamless switch(if supported) is possible only between video
> mode
>           * resolutions
>           */
>          if (src_mode->flags & DRM_MODE_FLAG_VIDEO_MODE &&
>                          dst_mode->flags & DRM_MODE_FLAG_VIDEO_MODE)
>                  return true;
>          else
>                  return false;
> 
>  }
> 
> 
> -- 
> Jeykumar S

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [RFC] Expanding drm_mode_modeinfo flags

[Daniel Vetter]

On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran wrote:
> On 2019-07-16 02:07, Daniel Vetter wrote:
> > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:
> > >     Hello All,
> > >     	drm_mode_modeinfo::flags is a 32 bit field currently used to
> > >     describe the properties of a connector mode. I see the least order
> > 22 bits
> > >     are already in use. Posting this RFC to discuss on any potential
> > plans to
> > >     expand the bit range support of this field for growing mode
> > properties and
> > >     ways to handle one such property needed by the msm dpu driver.
> > > 
> > >     msm drivers support panels which can dynamically switch between
> > >     video(active) and command(smart) modes. Within video mode, they
> > > also
> > support
> > >     switching between resolutions seamlessly i.e. glitch free
> > > resolution
> > switch.
> > >     But they cannot do a seamless switch from a resolutions from video
> > to
> > >     command or vice versa. Clients need to be aware for these
> > capablities before
> > >     they switch between the resolutions. Since these capabilities are
> > identified
> > >     per drm_mode, we are considering the below two approaches to
> > > handle
> > this
> > >     use case.
> > > 
> > >     Option 1:
> > >     Attached patch adds flag values to associate a drm_mode to
> > video/command
> > >     mode and to indicate its capability to do a seamless switch.
> > > 
> > >     Option 2:
> > >     drm_mode_modeinfo can expose a new "private_flags" field to handle
> > vendor
> > >     specific mode flags. Besides the above mentioned use case, we are
> > also
> > >     expoloring methods to handle some of our display port resolution
> > switch use
> > >     cases where the DP ports can be operated in a tiled/detiled modes.
> > This
> > >     approach will provide a standard channel for drm driver vendors
> > > for
> > their
> > >     growing need for drm_mode specific capabilities.
> > > 
> > >     Please provide your inputs on the options or any upstream friendly
> > >     recommendation to handle such custom use cases.
> > > 
> > >     Thanks and Regards,
> > >     Jeykumar S.
> > > 
> > > Jeykumar Sankaran (1):
> > >   drm: add mode flags in uapi for seamless mode switch
> > 
> > I think the uapi is the trivial part here, the real deal is how
> > userspace
> > uses this. Can you pls post the patches for your compositor?
> > 
> > Also note that we already allow userspace to tell the kernel whether
> > flickering is ok or not for a modeset. msm driver could use that to at
> > least tell userspace whether a modeset change is possible. So you can
> > already implement glitch-free modeset changes for at least video mode.
> > -Daniel
> 
> I believe you are referring to the below tv property of the connector.
> 
> /**
>  * @tv_flicker_reduction_property: Optional TV property to control the
>  * flicker reduction mode.
>  */
> struct drm_property *tv_flicker_reduction_property;

Not even close :-)

I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic ioctl. This
is not a property of a mode, this is a property of a _transition_ between
configurations. Some transitions can be done flicker free, others can't.

There's then still the question of how to pick video vs command mode, but
imo better to start with implementing the transition behaviour correctly
first.
-Daniel

> 
> Sure. We can use this to indicate whether the connector representing the
> panel
> can support the dynamic glitch-free switch. But when the panel supports both
> video and command mode resolutions and such glitch-free switch is possible
> only between
> video mode resolutions, we need per resolution(drm_mode_modeinfo)
> information
> to identify the resolutions enumerated for video mode.
> 
> Below is an example of the compositor utility function which can use the
> tv_flicker_property
> and the proposed modeinfo flags to identify glitch-free switches.
> 
>  bool is_seamless_switch_supported(struct drm_mode_modeinfo src_mode, struct
>                  drm_mode_modeinfo *dst_mode, bool
> flicker_reduction_supported)
>  {
>          if (!flicker_reduction_supported) {
>                  printf("flicker reduction prop not set for the
> connector\n");
>                  return false;
>          }
> 
>          /*
>           * Seamless switch(if supported) is possible only between video
> mode
>           * resolutions
>           */
>          if (src_mode->flags & DRM_MODE_FLAG_VIDEO_MODE &&
>                          dst_mode->flags & DRM_MODE_FLAG_VIDEO_MODE)
>                  return true;
>          else
>                  return false;
> 
>  }
> 
> 
> -- 
> Jeykumar S

-- 
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: [RFC] Expanding drm_mode_modeinfo flags

[Sean Paul]

On Fri, Jul 19, 2019 at 11:05:53AM +0200, Daniel Vetter wrote:
> On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran wrote:
> > On 2019-07-16 02:07, Daniel Vetter wrote:
> > > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:
> > > >     Hello All,
> > > >     	drm_mode_modeinfo::flags is a 32 bit field currently used to
> > > >     describe the properties of a connector mode. I see the least order
> > > 22 bits
> > > >     are already in use. Posting this RFC to discuss on any potential
> > > plans to
> > > >     expand the bit range support of this field for growing mode
> > > properties and
> > > >     ways to handle one such property needed by the msm dpu driver.
> > > > 
> > > >     msm drivers support panels which can dynamically switch between
> > > >     video(active) and command(smart) modes. Within video mode, they
> > > > also
> > > support
> > > >     switching between resolutions seamlessly i.e. glitch free
> > > > resolution
> > > switch.
> > > >     But they cannot do a seamless switch from a resolutions from video
> > > to
> > > >     command or vice versa. Clients need to be aware for these
> > > capablities before
> > > >     they switch between the resolutions. Since these capabilities are
> > > identified
> > > >     per drm_mode, we are considering the below two approaches to
> > > > handle
> > > this
> > > >     use case.
> > > > 
> > > >     Option 1:
> > > >     Attached patch adds flag values to associate a drm_mode to
> > > video/command
> > > >     mode and to indicate its capability to do a seamless switch.
> > > > 
> > > >     Option 2:
> > > >     drm_mode_modeinfo can expose a new "private_flags" field to handle
> > > vendor
> > > >     specific mode flags. Besides the above mentioned use case, we are
> > > also
> > > >     expoloring methods to handle some of our display port resolution
> > > switch use
> > > >     cases where the DP ports can be operated in a tiled/detiled modes.
> > > This
> > > >     approach will provide a standard channel for drm driver vendors
> > > > for
> > > their
> > > >     growing need for drm_mode specific capabilities.
> > > > 
> > > >     Please provide your inputs on the options or any upstream friendly
> > > >     recommendation to handle such custom use cases.
> > > > 
> > > >     Thanks and Regards,
> > > >     Jeykumar S.
> > > > 
> > > > Jeykumar Sankaran (1):
> > > >   drm: add mode flags in uapi for seamless mode switch
> > > 
> > > I think the uapi is the trivial part here, the real deal is how
> > > userspace
> > > uses this. Can you pls post the patches for your compositor?
> > > 
> > > Also note that we already allow userspace to tell the kernel whether
> > > flickering is ok or not for a modeset. msm driver could use that to at
> > > least tell userspace whether a modeset change is possible. So you can
> > > already implement glitch-free modeset changes for at least video mode.
> > > -Daniel
> > 
> > I believe you are referring to the below tv property of the connector.
> > 
> > /**
> >  * @tv_flicker_reduction_property: Optional TV property to control the
> >  * flicker reduction mode.
> >  */
> > struct drm_property *tv_flicker_reduction_property;
> 
> Not even close :-)
> 
> I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic ioctl. This
> is not a property of a mode, this is a property of a _transition_ between
> configurations. Some transitions can be done flicker free, others can't.

Agree that an atomic flag on a commit is the way to accomplish this. It's pretty
similar to the psr transitions, where we want to reuse most of the atomic
circuitry, but in a specialized way. We'd also have to be careful to only
involve the drm objects which are seamless modeset aware (you could imagine
a bridge chain where the bridges downstream of the first bridge don't care).

> 
> There's then still the question of how to pick video vs command mode, but
> imo better to start with implementing the transition behaviour correctly
> first.

Connector property? Possibly a terrible idea, but I wonder if we could [re]use
the vrr properties for command mode. The docs state that the driver has the
option of putting upper and lower bounds on the refresh rate.

Sean

> -Daniel
> 
> > 
> > Sure. We can use this to indicate whether the connector representing the
> > panel
> > can support the dynamic glitch-free switch. But when the panel supports both
> > video and command mode resolutions and such glitch-free switch is possible
> > only between
> > video mode resolutions, we need per resolution(drm_mode_modeinfo)
> > information
> > to identify the resolutions enumerated for video mode.
> > 
> > Below is an example of the compositor utility function which can use the
> > tv_flicker_property
> > and the proposed modeinfo flags to identify glitch-free switches.
> > 
> >  bool is_seamless_switch_supported(struct drm_mode_modeinfo src_mode, struct
> >                  drm_mode_modeinfo *dst_mode, bool
> > flicker_reduction_supported)
> >  {
> >          if (!flicker_reduction_supported) {
> >                  printf("flicker reduction prop not set for the
> > connector\n");
> >                  return false;
> >          }
> > 
> >          /*
> >           * Seamless switch(if supported) is possible only between video
> > mode
> >           * resolutions
> >           */
> >          if (src_mode->flags & DRM_MODE_FLAG_VIDEO_MODE &&
> >                          dst_mode->flags & DRM_MODE_FLAG_VIDEO_MODE)
> >                  return true;
> >          else
> >                  return false;
> > 
> >  }
> > 
> > 
> > -- 
> > Jeykumar S
> 
> -- 
> 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

-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [RFC] Expanding drm_mode_modeinfo flags

[Sean Paul]

On Fri, Jul 19, 2019 at 11:05:53AM +0200, Daniel Vetter wrote:
> On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran wrote:
> > On 2019-07-16 02:07, Daniel Vetter wrote:
> > > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:
> > > >     Hello All,
> > > >     	drm_mode_modeinfo::flags is a 32 bit field currently used to
> > > >     describe the properties of a connector mode. I see the least order
> > > 22 bits
> > > >     are already in use. Posting this RFC to discuss on any potential
> > > plans to
> > > >     expand the bit range support of this field for growing mode
> > > properties and
> > > >     ways to handle one such property needed by the msm dpu driver.
> > > > 
> > > >     msm drivers support panels which can dynamically switch between
> > > >     video(active) and command(smart) modes. Within video mode, they
> > > > also
> > > support
> > > >     switching between resolutions seamlessly i.e. glitch free
> > > > resolution
> > > switch.
> > > >     But they cannot do a seamless switch from a resolutions from video
> > > to
> > > >     command or vice versa. Clients need to be aware for these
> > > capablities before
> > > >     they switch between the resolutions. Since these capabilities are
> > > identified
> > > >     per drm_mode, we are considering the below two approaches to
> > > > handle
> > > this
> > > >     use case.
> > > > 
> > > >     Option 1:
> > > >     Attached patch adds flag values to associate a drm_mode to
> > > video/command
> > > >     mode and to indicate its capability to do a seamless switch.
> > > > 
> > > >     Option 2:
> > > >     drm_mode_modeinfo can expose a new "private_flags" field to handle
> > > vendor
> > > >     specific mode flags. Besides the above mentioned use case, we are
> > > also
> > > >     expoloring methods to handle some of our display port resolution
> > > switch use
> > > >     cases where the DP ports can be operated in a tiled/detiled modes.
> > > This
> > > >     approach will provide a standard channel for drm driver vendors
> > > > for
> > > their
> > > >     growing need for drm_mode specific capabilities.
> > > > 
> > > >     Please provide your inputs on the options or any upstream friendly
> > > >     recommendation to handle such custom use cases.
> > > > 
> > > >     Thanks and Regards,
> > > >     Jeykumar S.
> > > > 
> > > > Jeykumar Sankaran (1):
> > > >   drm: add mode flags in uapi for seamless mode switch
> > > 
> > > I think the uapi is the trivial part here, the real deal is how
> > > userspace
> > > uses this. Can you pls post the patches for your compositor?
> > > 
> > > Also note that we already allow userspace to tell the kernel whether
> > > flickering is ok or not for a modeset. msm driver could use that to at
> > > least tell userspace whether a modeset change is possible. So you can
> > > already implement glitch-free modeset changes for at least video mode.
> > > -Daniel
> > 
> > I believe you are referring to the below tv property of the connector.
> > 
> > /**
> >  * @tv_flicker_reduction_property: Optional TV property to control the
> >  * flicker reduction mode.
> >  */
> > struct drm_property *tv_flicker_reduction_property;
> 
> Not even close :-)
> 
> I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic ioctl. This
> is not a property of a mode, this is a property of a _transition_ between
> configurations. Some transitions can be done flicker free, others can't.

Agree that an atomic flag on a commit is the way to accomplish this. It's pretty
similar to the psr transitions, where we want to reuse most of the atomic
circuitry, but in a specialized way. We'd also have to be careful to only
involve the drm objects which are seamless modeset aware (you could imagine
a bridge chain where the bridges downstream of the first bridge don't care).

> 
> There's then still the question of how to pick video vs command mode, but
> imo better to start with implementing the transition behaviour correctly
> first.

Connector property? Possibly a terrible idea, but I wonder if we could [re]use
the vrr properties for command mode. The docs state that the driver has the
option of putting upper and lower bounds on the refresh rate.

Sean

> -Daniel
> 
> > 
> > Sure. We can use this to indicate whether the connector representing the
> > panel
> > can support the dynamic glitch-free switch. But when the panel supports both
> > video and command mode resolutions and such glitch-free switch is possible
> > only between
> > video mode resolutions, we need per resolution(drm_mode_modeinfo)
> > information
> > to identify the resolutions enumerated for video mode.
> > 
> > Below is an example of the compositor utility function which can use the
> > tv_flicker_property
> > and the proposed modeinfo flags to identify glitch-free switches.
> > 
> >  bool is_seamless_switch_supported(struct drm_mode_modeinfo src_mode, struct
> >                  drm_mode_modeinfo *dst_mode, bool
> > flicker_reduction_supported)
> >  {
> >          if (!flicker_reduction_supported) {
> >                  printf("flicker reduction prop not set for the
> > connector\n");
> >                  return false;
> >          }
> > 
> >          /*
> >           * Seamless switch(if supported) is possible only between video
> > mode
> >           * resolutions
> >           */
> >          if (src_mode->flags & DRM_MODE_FLAG_VIDEO_MODE &&
> >                          dst_mode->flags & DRM_MODE_FLAG_VIDEO_MODE)
> >                  return true;
> >          else
> >                  return false;
> > 
> >  }
> > 
> > 
> > -- 
> > Jeykumar S
> 
> -- 
> 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

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC] Expanding drm_mode_modeinfo flags

[Ville Syrjälä]

On Fri, Jul 19, 2019 at 09:55:58AM -0400, Sean Paul wrote:
> On Fri, Jul 19, 2019 at 11:05:53AM +0200, Daniel Vetter wrote:
> > On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran wrote:
> > > On 2019-07-16 02:07, Daniel Vetter wrote:
> > > > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:
> > > > >     Hello All,
> > > > >     	drm_mode_modeinfo::flags is a 32 bit field currently used to
> > > > >     describe the properties of a connector mode. I see the least order
> > > > 22 bits
> > > > >     are already in use. Posting this RFC to discuss on any potential
> > > > plans to
> > > > >     expand the bit range support of this field for growing mode
> > > > properties and
> > > > >     ways to handle one such property needed by the msm dpu driver.
> > > > > 
> > > > >     msm drivers support panels which can dynamically switch between
> > > > >     video(active) and command(smart) modes. Within video mode, they
> > > > > also
> > > > support
> > > > >     switching between resolutions seamlessly i.e. glitch free
> > > > > resolution
> > > > switch.
> > > > >     But they cannot do a seamless switch from a resolutions from video
> > > > to
> > > > >     command or vice versa. Clients need to be aware for these
> > > > capablities before
> > > > >     they switch between the resolutions. Since these capabilities are
> > > > identified
> > > > >     per drm_mode, we are considering the below two approaches to
> > > > > handle
> > > > this
> > > > >     use case.
> > > > > 
> > > > >     Option 1:
> > > > >     Attached patch adds flag values to associate a drm_mode to
> > > > video/command
> > > > >     mode and to indicate its capability to do a seamless switch.
> > > > > 
> > > > >     Option 2:
> > > > >     drm_mode_modeinfo can expose a new "private_flags" field to handle
> > > > vendor
> > > > >     specific mode flags. Besides the above mentioned use case, we are
> > > > also
> > > > >     expoloring methods to handle some of our display port resolution
> > > > switch use
> > > > >     cases where the DP ports can be operated in a tiled/detiled modes.
> > > > This
> > > > >     approach will provide a standard channel for drm driver vendors
> > > > > for
> > > > their
> > > > >     growing need for drm_mode specific capabilities.
> > > > > 
> > > > >     Please provide your inputs on the options or any upstream friendly
> > > > >     recommendation to handle such custom use cases.
> > > > > 
> > > > >     Thanks and Regards,
> > > > >     Jeykumar S.
> > > > > 
> > > > > Jeykumar Sankaran (1):
> > > > >   drm: add mode flags in uapi for seamless mode switch
> > > > 
> > > > I think the uapi is the trivial part here, the real deal is how
> > > > userspace
> > > > uses this. Can you pls post the patches for your compositor?
> > > > 
> > > > Also note that we already allow userspace to tell the kernel whether
> > > > flickering is ok or not for a modeset. msm driver could use that to at
> > > > least tell userspace whether a modeset change is possible. So you can
> > > > already implement glitch-free modeset changes for at least video mode.
> > > > -Daniel
> > > 
> > > I believe you are referring to the below tv property of the connector.
> > > 
> > > /**
> > >  * @tv_flicker_reduction_property: Optional TV property to control the
> > >  * flicker reduction mode.
> > >  */
> > > struct drm_property *tv_flicker_reduction_property;
> > 
> > Not even close :-)
> > 
> > I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic ioctl. This
> > is not a property of a mode, this is a property of a _transition_ between
> > configurations. Some transitions can be done flicker free, others can't.
> 
> Agree that an atomic flag on a commit is the way to accomplish this. It's pretty
> similar to the psr transitions, where we want to reuse most of the atomic
> circuitry, but in a specialized way. We'd also have to be careful to only
> involve the drm objects which are seamless modeset aware (you could imagine
> a bridge chain where the bridges downstream of the first bridge don't care).
> 
> > 
> > There's then still the question of how to pick video vs command mode, but
> > imo better to start with implementing the transition behaviour correctly
> > first.
> 
> Connector property? Possibly a terrible idea, but I wonder if we could [re]use
> the vrr properties for command mode. The docs state that the driver has the
> option of putting upper and lower bounds on the refresh rate.

Not really sure why this needs new props and whatnot. This is kinda what
the i915 "fastset" stuff already does:
1. userspace asks for something to be changed via atomic
2. driver calculates whether a modeset is actually required
3. atomic validates need_modeset() vs. DRM_MODE_ATOMIC_ALLOW_MODESET
4. if (need_modeset) heavyweight_commit() else lightweight_commit()

Ie. why should userspace really care about anything except the
"flickers are OK" vs. "flickers not wanted" thing?

Also what's the benefit of using video mode if your panel supportes
command mode? Can you turn off the memory in the panel and actually
save power that way? And if there is a benefit can't the driver just
automagically switch between the two based on how often things are
getting updated? That would match how eDP PSR already works.

-- 
Ville Syrjälä
Intel

Re: [RFC] Expanding drm_mode_modeinfo flags

[Ville Syrjälä]

On Fri, Jul 19, 2019 at 09:55:58AM -0400, Sean Paul wrote:
> On Fri, Jul 19, 2019 at 11:05:53AM +0200, Daniel Vetter wrote:
> > On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran wrote:
> > > On 2019-07-16 02:07, Daniel Vetter wrote:
> > > > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:
> > > > >     Hello All,
> > > > >     	drm_mode_modeinfo::flags is a 32 bit field currently used to
> > > > >     describe the properties of a connector mode. I see the least order
> > > > 22 bits
> > > > >     are already in use. Posting this RFC to discuss on any potential
> > > > plans to
> > > > >     expand the bit range support of this field for growing mode
> > > > properties and
> > > > >     ways to handle one such property needed by the msm dpu driver.
> > > > > 
> > > > >     msm drivers support panels which can dynamically switch between
> > > > >     video(active) and command(smart) modes. Within video mode, they
> > > > > also
> > > > support
> > > > >     switching between resolutions seamlessly i.e. glitch free
> > > > > resolution
> > > > switch.
> > > > >     But they cannot do a seamless switch from a resolutions from video
> > > > to
> > > > >     command or vice versa. Clients need to be aware for these
> > > > capablities before
> > > > >     they switch between the resolutions. Since these capabilities are
> > > > identified
> > > > >     per drm_mode, we are considering the below two approaches to
> > > > > handle
> > > > this
> > > > >     use case.
> > > > > 
> > > > >     Option 1:
> > > > >     Attached patch adds flag values to associate a drm_mode to
> > > > video/command
> > > > >     mode and to indicate its capability to do a seamless switch.
> > > > > 
> > > > >     Option 2:
> > > > >     drm_mode_modeinfo can expose a new "private_flags" field to handle
> > > > vendor
> > > > >     specific mode flags. Besides the above mentioned use case, we are
> > > > also
> > > > >     expoloring methods to handle some of our display port resolution
> > > > switch use
> > > > >     cases where the DP ports can be operated in a tiled/detiled modes.
> > > > This
> > > > >     approach will provide a standard channel for drm driver vendors
> > > > > for
> > > > their
> > > > >     growing need for drm_mode specific capabilities.
> > > > > 
> > > > >     Please provide your inputs on the options or any upstream friendly
> > > > >     recommendation to handle such custom use cases.
> > > > > 
> > > > >     Thanks and Regards,
> > > > >     Jeykumar S.
> > > > > 
> > > > > Jeykumar Sankaran (1):
> > > > >   drm: add mode flags in uapi for seamless mode switch
> > > > 
> > > > I think the uapi is the trivial part here, the real deal is how
> > > > userspace
> > > > uses this. Can you pls post the patches for your compositor?
> > > > 
> > > > Also note that we already allow userspace to tell the kernel whether
> > > > flickering is ok or not for a modeset. msm driver could use that to at
> > > > least tell userspace whether a modeset change is possible. So you can
> > > > already implement glitch-free modeset changes for at least video mode.
> > > > -Daniel
> > > 
> > > I believe you are referring to the below tv property of the connector.
> > > 
> > > /**
> > >  * @tv_flicker_reduction_property: Optional TV property to control the
> > >  * flicker reduction mode.
> > >  */
> > > struct drm_property *tv_flicker_reduction_property;
> > 
> > Not even close :-)
> > 
> > I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic ioctl. This
> > is not a property of a mode, this is a property of a _transition_ between
> > configurations. Some transitions can be done flicker free, others can't.
> 
> Agree that an atomic flag on a commit is the way to accomplish this. It's pretty
> similar to the psr transitions, where we want to reuse most of the atomic
> circuitry, but in a specialized way. We'd also have to be careful to only
> involve the drm objects which are seamless modeset aware (you could imagine
> a bridge chain where the bridges downstream of the first bridge don't care).
> 
> > 
> > There's then still the question of how to pick video vs command mode, but
> > imo better to start with implementing the transition behaviour correctly
> > first.
> 
> Connector property? Possibly a terrible idea, but I wonder if we could [re]use
> the vrr properties for command mode. The docs state that the driver has the
> option of putting upper and lower bounds on the refresh rate.

Not really sure why this needs new props and whatnot. This is kinda what
the i915 "fastset" stuff already does:
1. userspace asks for something to be changed via atomic
2. driver calculates whether a modeset is actually required
3. atomic validates need_modeset() vs. DRM_MODE_ATOMIC_ALLOW_MODESET
4. if (need_modeset) heavyweight_commit() else lightweight_commit()

Ie. why should userspace really care about anything except the
"flickers are OK" vs. "flickers not wanted" thing?

Also what's the benefit of using video mode if your panel supportes
command mode? Can you turn off the memory in the panel and actually
save power that way? And if there is a benefit can't the driver just
automagically switch between the two based on how often things are
getting updated? That would match how eDP PSR already works.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC] Expanding drm_mode_modeinfo flags

[Sean Paul]

On Fri, Jul 19, 2019 at 05:15:28PM +0300, Ville Syrjälä wrote:
> On Fri, Jul 19, 2019 at 09:55:58AM -0400, Sean Paul wrote:
> > On Fri, Jul 19, 2019 at 11:05:53AM +0200, Daniel Vetter wrote:
> > > On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran wrote:
> > > > On 2019-07-16 02:07, Daniel Vetter wrote:
> > > > > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:

/snip

> > > > > >   drm: add mode flags in uapi for seamless mode switch
> > > > > 
> > > > > I think the uapi is the trivial part here, the real deal is how
> > > > > userspace
> > > > > uses this. Can you pls post the patches for your compositor?
> > > > > 
> > > > > Also note that we already allow userspace to tell the kernel whether
> > > > > flickering is ok or not for a modeset. msm driver could use that to at
> > > > > least tell userspace whether a modeset change is possible. So you can
> > > > > already implement glitch-free modeset changes for at least video mode.
> > > > > -Daniel
> > > > 
> > > > I believe you are referring to the below tv property of the connector.
> > > > 
> > > > /**
> > > >  * @tv_flicker_reduction_property: Optional TV property to control the
> > > >  * flicker reduction mode.
> > > >  */
> > > > struct drm_property *tv_flicker_reduction_property;
> > > 
> > > Not even close :-)
> > > 
> > > I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic ioctl. This
> > > is not a property of a mode, this is a property of a _transition_ between
> > > configurations. Some transitions can be done flicker free, others can't.
> > 
> > Agree that an atomic flag on a commit is the way to accomplish this. It's pretty
> > similar to the psr transitions, where we want to reuse most of the atomic
> > circuitry, but in a specialized way. We'd also have to be careful to only
> > involve the drm objects which are seamless modeset aware (you could imagine
> > a bridge chain where the bridges downstream of the first bridge don't care).
> > 
> > > 
> > > There's then still the question of how to pick video vs command mode, but
> > > imo better to start with implementing the transition behaviour correctly
> > > first.
> > 
> > Connector property? Possibly a terrible idea, but I wonder if we could [re]use
> > the vrr properties for command mode. The docs state that the driver has the
> > option of putting upper and lower bounds on the refresh rate.
> 
> Not really sure why this needs new props and whatnot. This is kinda what
> the i915 "fastset" stuff already does:
> 1. userspace asks for something to be changed via atomic
> 2. driver calculates whether a modeset is actually required
> 3. atomic validates need_modeset() vs. DRM_MODE_ATOMIC_ALLOW_MODESET
> 4. if (need_modeset) heavyweight_commit() else lightweight_commit()
> 
> Ie. why should userspace really care about anything except the
> "flickers are OK" vs. "flickers not wanted" thing?

Agree, I don't think the seamless modeset (ie: changing resolution without
flicker) needs a property. Just need to test the commit without ALLOW_MODESET
and commit it if the test passes.

> 
> Also what's the benefit of using video mode if your panel supportes
> command mode? Can you turn off the memory in the panel and actually
> save power that way? And if there is a benefit can't the driver just
> automagically switch between the two based on how often things are
> getting updated? That would match how eDP PSR already works.

I'm guessing video mode might have some latency benefits over command mode?

Sean

> 
> -- 
> Ville Syrjälä
> Intel

-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [RFC] Expanding drm_mode_modeinfo flags

[Sean Paul]

On Fri, Jul 19, 2019 at 05:15:28PM +0300, Ville Syrjälä wrote:
> On Fri, Jul 19, 2019 at 09:55:58AM -0400, Sean Paul wrote:
> > On Fri, Jul 19, 2019 at 11:05:53AM +0200, Daniel Vetter wrote:
> > > On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran wrote:
> > > > On 2019-07-16 02:07, Daniel Vetter wrote:
> > > > > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:

/snip

> > > > > >   drm: add mode flags in uapi for seamless mode switch
> > > > > 
> > > > > I think the uapi is the trivial part here, the real deal is how
> > > > > userspace
> > > > > uses this. Can you pls post the patches for your compositor?
> > > > > 
> > > > > Also note that we already allow userspace to tell the kernel whether
> > > > > flickering is ok or not for a modeset. msm driver could use that to at
> > > > > least tell userspace whether a modeset change is possible. So you can
> > > > > already implement glitch-free modeset changes for at least video mode.
> > > > > -Daniel
> > > > 
> > > > I believe you are referring to the below tv property of the connector.
> > > > 
> > > > /**
> > > >  * @tv_flicker_reduction_property: Optional TV property to control the
> > > >  * flicker reduction mode.
> > > >  */
> > > > struct drm_property *tv_flicker_reduction_property;
> > > 
> > > Not even close :-)
> > > 
> > > I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic ioctl. This
> > > is not a property of a mode, this is a property of a _transition_ between
> > > configurations. Some transitions can be done flicker free, others can't.
> > 
> > Agree that an atomic flag on a commit is the way to accomplish this. It's pretty
> > similar to the psr transitions, where we want to reuse most of the atomic
> > circuitry, but in a specialized way. We'd also have to be careful to only
> > involve the drm objects which are seamless modeset aware (you could imagine
> > a bridge chain where the bridges downstream of the first bridge don't care).
> > 
> > > 
> > > There's then still the question of how to pick video vs command mode, but
> > > imo better to start with implementing the transition behaviour correctly
> > > first.
> > 
> > Connector property? Possibly a terrible idea, but I wonder if we could [re]use
> > the vrr properties for command mode. The docs state that the driver has the
> > option of putting upper and lower bounds on the refresh rate.
> 
> Not really sure why this needs new props and whatnot. This is kinda what
> the i915 "fastset" stuff already does:
> 1. userspace asks for something to be changed via atomic
> 2. driver calculates whether a modeset is actually required
> 3. atomic validates need_modeset() vs. DRM_MODE_ATOMIC_ALLOW_MODESET
> 4. if (need_modeset) heavyweight_commit() else lightweight_commit()
> 
> Ie. why should userspace really care about anything except the
> "flickers are OK" vs. "flickers not wanted" thing?

Agree, I don't think the seamless modeset (ie: changing resolution without
flicker) needs a property. Just need to test the commit without ALLOW_MODESET
and commit it if the test passes.

> 
> Also what's the benefit of using video mode if your panel supportes
> command mode? Can you turn off the memory in the panel and actually
> save power that way? And if there is a benefit can't the driver just
> automagically switch between the two based on how often things are
> getting updated? That would match how eDP PSR already works.

I'm guessing video mode might have some latency benefits over command mode?

Sean

> 
> -- 
> Ville Syrjälä
> Intel

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [RFC] Expanding drm_mode_modeinfo flags

[Jeykumar Sankaran]

On 2019-07-19 07:29, Sean Paul wrote:
> On Fri, Jul 19, 2019 at 05:15:28PM +0300, Ville Syrjälä wrote:
>> On Fri, Jul 19, 2019 at 09:55:58AM -0400, Sean Paul wrote:
>> > On Fri, Jul 19, 2019 at 11:05:53AM +0200, Daniel Vetter wrote:
>> > > On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran wrote:
>> > > > On 2019-07-16 02:07, Daniel Vetter wrote:
>> > > > > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:
> 
> /snip
> 
>> > > > > >   drm: add mode flags in uapi for seamless mode switch
>> > > > >
>> > > > > I think the uapi is the trivial part here, the real deal is how
>> > > > > userspace
>> > > > > uses this. Can you pls post the patches for your compositor?
>> > > > >
>> > > > > Also note that we already allow userspace to tell the kernel whether
>> > > > > flickering is ok or not for a modeset. msm driver could use that to at
>> > > > > least tell userspace whether a modeset change is possible. So you can
>> > > > > already implement glitch-free modeset changes for at least video mode.
>> > > > > -Daniel
>> > > >
>> > > > I believe you are referring to the below tv property of the connector.
>> > > >
>> > > > /**
>> > > >  * @tv_flicker_reduction_property: Optional TV property to control the
>> > > >  * flicker reduction mode.
>> > > >  */
>> > > > struct drm_property *tv_flicker_reduction_property;
>> > >
>> > > Not even close :-)
>> > >
>> > > I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic ioctl. This
>> > > is not a property of a mode, this is a property of a _transition_ between
>> > > configurations. Some transitions can be done flicker free, others can't.
>> >
>> > Agree that an atomic flag on a commit is the way to accomplish this. It's pretty
>> > similar to the psr transitions, where we want to reuse most of the atomic
>> > circuitry, but in a specialized way. We'd also have to be careful to only
>> > involve the drm objects which are seamless modeset aware (you could imagine
>> > a bridge chain where the bridges downstream of the first bridge don't care).
>> >
>> > >
>> > > There's then still the question of how to pick video vs command mode, but
>> > > imo better to start with implementing the transition behaviour correctly
>> > > first.
>> >
>> > Connector property? Possibly a terrible idea, but I wonder if we could [re]use
>> > the vrr properties for command mode. The docs state that the driver has the
>> > option of putting upper and lower bounds on the refresh rate.
>> 
>> Not really sure why this needs new props and whatnot. This is kinda 
>> what
>> the i915 "fastset" stuff already does:
>> 1. userspace asks for something to be changed via atomic
>> 2. driver calculates whether a modeset is actually required
>> 3. atomic validates need_modeset() vs. DRM_MODE_ATOMIC_ALLOW_MODESET
>> 4. if (need_modeset) heavyweight_commit() else lightweight_commit()
>> 
>> Ie. why should userspace really care about anything except the
>> "flickers are OK" vs. "flickers not wanted" thing?
> 
> Agree, I don't think the seamless modeset (ie: changing resolution 
> without
> flicker) needs a property. Just need to test the commit without 
> ALLOW_MODESET
> and commit it if the test passes.
> 

Agreed that a TEST_ONLY commit without ALLOW_MODESET flag can be used to 
check
whether the modeset can be done seamless. But since there are no error 
code returns,
the client cannot distinguish the test_only commit failures from other 
invalid config failures.

Also, note that when the client sees two 1080p modes (vid/cmd) and it is 
interested only
to do *only* seamless switches, without any additional flag it cannot 
distinguish between
these two 1080p modes. The client has to invoke two test_only commits 
with these
modes to identify the seamless one. Is that a preferred approach?

Intel's "fastset" calculates the need for modeset internally based on 
the
configuration and chooses the best commit path. But the requirement here
is to expose the information up-front since the use case cannot afford
to fall back to the normal modeset when it has requested for a seamless 
one.

>> 
>> Also what's the benefit of using video mode if your panel supportes
>> command mode? Can you turn off the memory in the panel and actually
>> save power that way? And if there is a benefit can't the driver just
>> automagically switch between the two based on how often things are
>> getting updated? That would match how eDP PSR already works.
> 
> I'm guessing video mode might have some latency benefits over command 
> mode?
> 
> Sean

Yes. Pretty much those are reasons we need to switch to video mode. But 
instead
of making the decision internal to the driver based on the frequency of 
frame updates,
we have proprietary use cases where the client has to trigger the switch 
explicitly.
So we are trying to find ways to represent the same resolution in both 
video/cmd modes.

Thanks and Regards,
Jeykumar S.

> 
>> 
>> --
>> Ville Syrjälä
>> Intel

-- 
Jeykumar S

Re: [RFC] Expanding drm_mode_modeinfo flags

[Jeykumar Sankaran]

On 2019-07-19 07:29, Sean Paul wrote:
> On Fri, Jul 19, 2019 at 05:15:28PM +0300, Ville Syrjälä wrote:
>> On Fri, Jul 19, 2019 at 09:55:58AM -0400, Sean Paul wrote:
>> > On Fri, Jul 19, 2019 at 11:05:53AM +0200, Daniel Vetter wrote:
>> > > On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran wrote:
>> > > > On 2019-07-16 02:07, Daniel Vetter wrote:
>> > > > > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:
> 
> /snip
> 
>> > > > > >   drm: add mode flags in uapi for seamless mode switch
>> > > > >
>> > > > > I think the uapi is the trivial part here, the real deal is how
>> > > > > userspace
>> > > > > uses this. Can you pls post the patches for your compositor?
>> > > > >
>> > > > > Also note that we already allow userspace to tell the kernel whether
>> > > > > flickering is ok or not for a modeset. msm driver could use that to at
>> > > > > least tell userspace whether a modeset change is possible. So you can
>> > > > > already implement glitch-free modeset changes for at least video mode.
>> > > > > -Daniel
>> > > >
>> > > > I believe you are referring to the below tv property of the connector.
>> > > >
>> > > > /**
>> > > >  * @tv_flicker_reduction_property: Optional TV property to control the
>> > > >  * flicker reduction mode.
>> > > >  */
>> > > > struct drm_property *tv_flicker_reduction_property;
>> > >
>> > > Not even close :-)
>> > >
>> > > I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic ioctl. This
>> > > is not a property of a mode, this is a property of a _transition_ between
>> > > configurations. Some transitions can be done flicker free, others can't.
>> >
>> > Agree that an atomic flag on a commit is the way to accomplish this. It's pretty
>> > similar to the psr transitions, where we want to reuse most of the atomic
>> > circuitry, but in a specialized way. We'd also have to be careful to only
>> > involve the drm objects which are seamless modeset aware (you could imagine
>> > a bridge chain where the bridges downstream of the first bridge don't care).
>> >
>> > >
>> > > There's then still the question of how to pick video vs command mode, but
>> > > imo better to start with implementing the transition behaviour correctly
>> > > first.
>> >
>> > Connector property? Possibly a terrible idea, but I wonder if we could [re]use
>> > the vrr properties for command mode. The docs state that the driver has the
>> > option of putting upper and lower bounds on the refresh rate.
>> 
>> Not really sure why this needs new props and whatnot. This is kinda 
>> what
>> the i915 "fastset" stuff already does:
>> 1. userspace asks for something to be changed via atomic
>> 2. driver calculates whether a modeset is actually required
>> 3. atomic validates need_modeset() vs. DRM_MODE_ATOMIC_ALLOW_MODESET
>> 4. if (need_modeset) heavyweight_commit() else lightweight_commit()
>> 
>> Ie. why should userspace really care about anything except the
>> "flickers are OK" vs. "flickers not wanted" thing?
> 
> Agree, I don't think the seamless modeset (ie: changing resolution 
> without
> flicker) needs a property. Just need to test the commit without 
> ALLOW_MODESET
> and commit it if the test passes.
> 

Agreed that a TEST_ONLY commit without ALLOW_MODESET flag can be used to 
check
whether the modeset can be done seamless. But since there are no error 
code returns,
the client cannot distinguish the test_only commit failures from other 
invalid config failures.

Also, note that when the client sees two 1080p modes (vid/cmd) and it is 
interested only
to do *only* seamless switches, without any additional flag it cannot 
distinguish between
these two 1080p modes. The client has to invoke two test_only commits 
with these
modes to identify the seamless one. Is that a preferred approach?

Intel's "fastset" calculates the need for modeset internally based on 
the
configuration and chooses the best commit path. But the requirement here
is to expose the information up-front since the use case cannot afford
to fall back to the normal modeset when it has requested for a seamless 
one.

>> 
>> Also what's the benefit of using video mode if your panel supportes
>> command mode? Can you turn off the memory in the panel and actually
>> save power that way? And if there is a benefit can't the driver just
>> automagically switch between the two based on how often things are
>> getting updated? That would match how eDP PSR already works.
> 
> I'm guessing video mode might have some latency benefits over command 
> mode?
> 
> Sean

Yes. Pretty much those are reasons we need to switch to video mode. But 
instead
of making the decision internal to the driver based on the frequency of 
frame updates,
we have proprietary use cases where the client has to trigger the switch 
explicitly.
So we are trying to find ways to represent the same resolution in both 
video/cmd modes.

Thanks and Regards,
Jeykumar S.

> 
>> 
>> --
>> Ville Syrjälä
>> Intel

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

Re: [RFC] Expanding drm_mode_modeinfo flags

[Daniel Vetter]

On Tue, Jul 23, 2019 at 1:50 AM Jeykumar Sankaran <jsanka@codeaurora.org> wrote:
>
> On 2019-07-19 07:29, Sean Paul wrote:
> > On Fri, Jul 19, 2019 at 05:15:28PM +0300, Ville Syrjälä wrote:
> >> On Fri, Jul 19, 2019 at 09:55:58AM -0400, Sean Paul wrote:
> >> > On Fri, Jul 19, 2019 at 11:05:53AM +0200, Daniel Vetter wrote:
> >> > > On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran wrote:
> >> > > > On 2019-07-16 02:07, Daniel Vetter wrote:
> >> > > > > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:
> >
> > /snip
> >
> >> > > > > >   drm: add mode flags in uapi for seamless mode switch
> >> > > > >
> >> > > > > I think the uapi is the trivial part here, the real deal is how
> >> > > > > userspace
> >> > > > > uses this. Can you pls post the patches for your compositor?
> >> > > > >
> >> > > > > Also note that we already allow userspace to tell the kernel whether
> >> > > > > flickering is ok or not for a modeset. msm driver could use that to at
> >> > > > > least tell userspace whether a modeset change is possible. So you can
> >> > > > > already implement glitch-free modeset changes for at least video mode.
> >> > > > > -Daniel
> >> > > >
> >> > > > I believe you are referring to the below tv property of the connector.
> >> > > >
> >> > > > /**
> >> > > >  * @tv_flicker_reduction_property: Optional TV property to control the
> >> > > >  * flicker reduction mode.
> >> > > >  */
> >> > > > struct drm_property *tv_flicker_reduction_property;
> >> > >
> >> > > Not even close :-)
> >> > >
> >> > > I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic ioctl. This
> >> > > is not a property of a mode, this is a property of a _transition_ between
> >> > > configurations. Some transitions can be done flicker free, others can't.
> >> >
> >> > Agree that an atomic flag on a commit is the way to accomplish this. It's pretty
> >> > similar to the psr transitions, where we want to reuse most of the atomic
> >> > circuitry, but in a specialized way. We'd also have to be careful to only
> >> > involve the drm objects which are seamless modeset aware (you could imagine
> >> > a bridge chain where the bridges downstream of the first bridge don't care).
> >> >
> >> > >
> >> > > There's then still the question of how to pick video vs command mode, but
> >> > > imo better to start with implementing the transition behaviour correctly
> >> > > first.
> >> >
> >> > Connector property? Possibly a terrible idea, but I wonder if we could [re]use
> >> > the vrr properties for command mode. The docs state that the driver has the
> >> > option of putting upper and lower bounds on the refresh rate.
> >>
> >> Not really sure why this needs new props and whatnot. This is kinda
> >> what
> >> the i915 "fastset" stuff already does:
> >> 1. userspace asks for something to be changed via atomic
> >> 2. driver calculates whether a modeset is actually required
> >> 3. atomic validates need_modeset() vs. DRM_MODE_ATOMIC_ALLOW_MODESET
> >> 4. if (need_modeset) heavyweight_commit() else lightweight_commit()
> >>
> >> Ie. why should userspace really care about anything except the
> >> "flickers are OK" vs. "flickers not wanted" thing?
> >
> > Agree, I don't think the seamless modeset (ie: changing resolution
> > without
> > flicker) needs a property. Just need to test the commit without
> > ALLOW_MODESET
> > and commit it if the test passes.
> >
>
> Agreed that a TEST_ONLY commit without ALLOW_MODESET flag can be used to
> check
> whether the modeset can be done seamless. But since there are no error
> code returns,
> the client cannot distinguish the test_only commit failures from other
> invalid config failures.
>
> Also, note that when the client sees two 1080p modes (vid/cmd) and it is
> interested only
> to do *only* seamless switches, without any additional flag it cannot
> distinguish between
> these two 1080p modes. The client has to invoke two test_only commits
> with these
> modes to identify the seamless one. Is that a preferred approach?
>
> Intel's "fastset" calculates the need for modeset internally based on
> the
> configuration and chooses the best commit path. But the requirement here
> is to expose the information up-front since the use case cannot afford
> to fall back to the normal modeset when it has requested for a seamless
> one.
>
> >>
> >> Also what's the benefit of using video mode if your panel supportes
> >> command mode? Can you turn off the memory in the panel and actually
> >> save power that way? And if there is a benefit can't the driver just
> >> automagically switch between the two based on how often things are
> >> getting updated? That would match how eDP PSR already works.
> >
> > I'm guessing video mode might have some latency benefits over command
> > mode?
> >
> > Sean
>
> Yes. Pretty much those are reasons we need to switch to video mode. But
> instead
> of making the decision internal to the driver based on the frequency of
> frame updates,
> we have proprietary use cases where the client has to trigger the switch
> explicitly.
> So we are trying to find ways to represent the same resolution in both
> video/cmd modes.

If "proprietary" here means closed source or not upstream, then that's
the part you need to fix first. See

https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements

Cheers, Daniel

>
> Thanks and Regards,
> Jeykumar S.
>
> >
> >>
> >> --
> >> Ville Syrjälä
> >> Intel
>
> --
> Jeykumar S



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [RFC] Expanding drm_mode_modeinfo flags

[Daniel Vetter]

On Tue, Jul 23, 2019 at 1:50 AM Jeykumar Sankaran <jsanka@codeaurora.org> wrote:
>
> On 2019-07-19 07:29, Sean Paul wrote:
> > On Fri, Jul 19, 2019 at 05:15:28PM +0300, Ville Syrjälä wrote:
> >> On Fri, Jul 19, 2019 at 09:55:58AM -0400, Sean Paul wrote:
> >> > On Fri, Jul 19, 2019 at 11:05:53AM +0200, Daniel Vetter wrote:
> >> > > On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran wrote:
> >> > > > On 2019-07-16 02:07, Daniel Vetter wrote:
> >> > > > > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:
> >
> > /snip
> >
> >> > > > > >   drm: add mode flags in uapi for seamless mode switch
> >> > > > >
> >> > > > > I think the uapi is the trivial part here, the real deal is how
> >> > > > > userspace
> >> > > > > uses this. Can you pls post the patches for your compositor?
> >> > > > >
> >> > > > > Also note that we already allow userspace to tell the kernel whether
> >> > > > > flickering is ok or not for a modeset. msm driver could use that to at
> >> > > > > least tell userspace whether a modeset change is possible. So you can
> >> > > > > already implement glitch-free modeset changes for at least video mode.
> >> > > > > -Daniel
> >> > > >
> >> > > > I believe you are referring to the below tv property of the connector.
> >> > > >
> >> > > > /**
> >> > > >  * @tv_flicker_reduction_property: Optional TV property to control the
> >> > > >  * flicker reduction mode.
> >> > > >  */
> >> > > > struct drm_property *tv_flicker_reduction_property;
> >> > >
> >> > > Not even close :-)
> >> > >
> >> > > I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic ioctl. This
> >> > > is not a property of a mode, this is a property of a _transition_ between
> >> > > configurations. Some transitions can be done flicker free, others can't.
> >> >
> >> > Agree that an atomic flag on a commit is the way to accomplish this. It's pretty
> >> > similar to the psr transitions, where we want to reuse most of the atomic
> >> > circuitry, but in a specialized way. We'd also have to be careful to only
> >> > involve the drm objects which are seamless modeset aware (you could imagine
> >> > a bridge chain where the bridges downstream of the first bridge don't care).
> >> >
> >> > >
> >> > > There's then still the question of how to pick video vs command mode, but
> >> > > imo better to start with implementing the transition behaviour correctly
> >> > > first.
> >> >
> >> > Connector property? Possibly a terrible idea, but I wonder if we could [re]use
> >> > the vrr properties for command mode. The docs state that the driver has the
> >> > option of putting upper and lower bounds on the refresh rate.
> >>
> >> Not really sure why this needs new props and whatnot. This is kinda
> >> what
> >> the i915 "fastset" stuff already does:
> >> 1. userspace asks for something to be changed via atomic
> >> 2. driver calculates whether a modeset is actually required
> >> 3. atomic validates need_modeset() vs. DRM_MODE_ATOMIC_ALLOW_MODESET
> >> 4. if (need_modeset) heavyweight_commit() else lightweight_commit()
> >>
> >> Ie. why should userspace really care about anything except the
> >> "flickers are OK" vs. "flickers not wanted" thing?
> >
> > Agree, I don't think the seamless modeset (ie: changing resolution
> > without
> > flicker) needs a property. Just need to test the commit without
> > ALLOW_MODESET
> > and commit it if the test passes.
> >
>
> Agreed that a TEST_ONLY commit without ALLOW_MODESET flag can be used to
> check
> whether the modeset can be done seamless. But since there are no error
> code returns,
> the client cannot distinguish the test_only commit failures from other
> invalid config failures.
>
> Also, note that when the client sees two 1080p modes (vid/cmd) and it is
> interested only
> to do *only* seamless switches, without any additional flag it cannot
> distinguish between
> these two 1080p modes. The client has to invoke two test_only commits
> with these
> modes to identify the seamless one. Is that a preferred approach?
>
> Intel's "fastset" calculates the need for modeset internally based on
> the
> configuration and chooses the best commit path. But the requirement here
> is to expose the information up-front since the use case cannot afford
> to fall back to the normal modeset when it has requested for a seamless
> one.
>
> >>
> >> Also what's the benefit of using video mode if your panel supportes
> >> command mode? Can you turn off the memory in the panel and actually
> >> save power that way? And if there is a benefit can't the driver just
> >> automagically switch between the two based on how often things are
> >> getting updated? That would match how eDP PSR already works.
> >
> > I'm guessing video mode might have some latency benefits over command
> > mode?
> >
> > Sean
>
> Yes. Pretty much those are reasons we need to switch to video mode. But
> instead
> of making the decision internal to the driver based on the frequency of
> frame updates,
> we have proprietary use cases where the client has to trigger the switch
> explicitly.
> So we are trying to find ways to represent the same resolution in both
> video/cmd modes.

If "proprietary" here means closed source or not upstream, then that's
the part you need to fix first. See

https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements

Cheers, Daniel

>
> Thanks and Regards,
> Jeykumar S.
>
> >
> >>
> >> --
> >> Ville Syrjälä
> >> Intel
>
> --
> Jeykumar S



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC] Expanding drm_mode_modeinfo flags

[Sean Paul]

On Mon, Jul 22, 2019 at 04:50:43PM -0700, Jeykumar Sankaran wrote:
> On 2019-07-19 07:29, Sean Paul wrote:
> > On Fri, Jul 19, 2019 at 05:15:28PM +0300, Ville Syrjälä wrote:
> > > On Fri, Jul 19, 2019 at 09:55:58AM -0400, Sean Paul wrote:
> > > > On Fri, Jul 19, 2019 at 11:05:53AM +0200, Daniel Vetter wrote:
> > > > > On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran wrote:
> > > > > > On 2019-07-16 02:07, Daniel Vetter wrote:
> > > > > > > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:
> > 
> > /snip
> > 
> > > > > > > >   drm: add mode flags in uapi for seamless mode switch
> > > > > > >
> > > > > > > I think the uapi is the trivial part here, the real deal is how
> > > > > > > userspace
> > > > > > > uses this. Can you pls post the patches for your compositor?
> > > > > > >
> > > > > > > Also note that we already allow userspace to tell the kernel whether
> > > > > > > flickering is ok or not for a modeset. msm driver could use that to at
> > > > > > > least tell userspace whether a modeset change is possible. So you can
> > > > > > > already implement glitch-free modeset changes for at least video mode.
> > > > > > > -Daniel
> > > > > >
> > > > > > I believe you are referring to the below tv property of the connector.
> > > > > >
> > > > > > /**
> > > > > >  * @tv_flicker_reduction_property: Optional TV property to control the
> > > > > >  * flicker reduction mode.
> > > > > >  */
> > > > > > struct drm_property *tv_flicker_reduction_property;
> > > > >
> > > > > Not even close :-)
> > > > >
> > > > > I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic ioctl. This
> > > > > is not a property of a mode, this is a property of a _transition_ between
> > > > > configurations. Some transitions can be done flicker free, others can't.
> > > >
> > > > Agree that an atomic flag on a commit is the way to accomplish this. It's pretty
> > > > similar to the psr transitions, where we want to reuse most of the atomic
> > > > circuitry, but in a specialized way. We'd also have to be careful to only
> > > > involve the drm objects which are seamless modeset aware (you could imagine
> > > > a bridge chain where the bridges downstream of the first bridge don't care).
> > > >
> > > > >
> > > > > There's then still the question of how to pick video vs command mode, but
> > > > > imo better to start with implementing the transition behaviour correctly
> > > > > first.
> > > >
> > > > Connector property? Possibly a terrible idea, but I wonder if we could [re]use
> > > > the vrr properties for command mode. The docs state that the driver has the
> > > > option of putting upper and lower bounds on the refresh rate.
> > > 
> > > Not really sure why this needs new props and whatnot. This is kinda
> > > what
> > > the i915 "fastset" stuff already does:
> > > 1. userspace asks for something to be changed via atomic
> > > 2. driver calculates whether a modeset is actually required
> > > 3. atomic validates need_modeset() vs. DRM_MODE_ATOMIC_ALLOW_MODESET
> > > 4. if (need_modeset) heavyweight_commit() else lightweight_commit()
> > > 
> > > Ie. why should userspace really care about anything except the
> > > "flickers are OK" vs. "flickers not wanted" thing?
> > 
> > Agree, I don't think the seamless modeset (ie: changing resolution
> > without
> > flicker) needs a property. Just need to test the commit without
> > ALLOW_MODESET
> > and commit it if the test passes.
> > 
> 
> Agreed that a TEST_ONLY commit without ALLOW_MODESET flag can be used to
> check
> whether the modeset can be done seamless. But since there are no error code
> returns,
> the client cannot distinguish the test_only commit failures from other
> invalid config failures.
> 
> Also, note that when the client sees two 1080p modes (vid/cmd) and it is
> interested only
> to do *only* seamless switches, without any additional flag it cannot
> distinguish between
> these two 1080p modes. The client has to invoke two test_only commits with
> these
> modes to identify the seamless one. Is that a preferred approach?

Hi Jey!
Yeah, pretty much. Stepping back a bit though, why is the kernel exposing 2
1080p modes in the first place? If you just expose one mode and then use a
property to enter "low-latency operation" (or overloading vrr for cmd mode), you
shouldn't need to do the mode switch, just flip the property and let the kernel
figure out how to transition to video/cmd mode.

> 
> Intel's "fastset" calculates the need for modeset internally based on the
> configuration and chooses the best commit path. But the requirement here
> is to expose the information up-front since the use case cannot afford
> to fall back to the normal modeset when it has requested for a seamless one.
> 
> > > 
> > > Also what's the benefit of using video mode if your panel supportes
> > > command mode? Can you turn off the memory in the panel and actually
> > > save power that way? And if there is a benefit can't the driver just
> > > automagically switch between the two based on how often things are
> > > getting updated? That would match how eDP PSR already works.
> > 
> > I'm guessing video mode might have some latency benefits over command
> > mode?
> > 
> > Sean
> 
> Yes. Pretty much those are reasons we need to switch to video mode. But
> instead
> of making the decision internal to the driver based on the frequency of
> frame updates,
> we have proprietary use cases where the client has to trigger the switch
> explicitly.

Unsolicited advice: if you find yourself typing "proprietary" while justifying
an upstream proposal, reword your argument immediately :-)

Now that I've filled my awful joke quota for the mail, I can move on. Generally
speaking, it's better to you expand on why userspace needs this. There's a very
good chance that if it makes sense for Qualcomm, it makes sense for others. If
we have an open discussion about it, others might chime in with "me too" and
that will help your case (and you might even hook someone into doing the work
for you!). Alternatively, we might be able to solve the problem in a different
way that makes everyone happy (or most everyone happy).

There's 100% a case for allowing userspace to trigger a low-latency mode at the
expense of power (we should have it disable the psr entry timer as well). Let's
steer this conversation in the direction of "can we use something that already
exists, or should we add something new?"

Sean


> So we are trying to find ways to represent the same resolution in both
> video/cmd modes.
> 
> Thanks and Regards,
> Jeykumar S.
> 
> > 
> > > 
> > > --
> > > Ville Syrjälä
> > > Intel
> 
> -- 
> Jeykumar S

-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [RFC] Expanding drm_mode_modeinfo flags

[Sean Paul]

On Mon, Jul 22, 2019 at 04:50:43PM -0700, Jeykumar Sankaran wrote:
> On 2019-07-19 07:29, Sean Paul wrote:
> > On Fri, Jul 19, 2019 at 05:15:28PM +0300, Ville Syrjälä wrote:
> > > On Fri, Jul 19, 2019 at 09:55:58AM -0400, Sean Paul wrote:
> > > > On Fri, Jul 19, 2019 at 11:05:53AM +0200, Daniel Vetter wrote:
> > > > > On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran wrote:
> > > > > > On 2019-07-16 02:07, Daniel Vetter wrote:
> > > > > > > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:
> > 
> > /snip
> > 
> > > > > > > >   drm: add mode flags in uapi for seamless mode switch
> > > > > > >
> > > > > > > I think the uapi is the trivial part here, the real deal is how
> > > > > > > userspace
> > > > > > > uses this. Can you pls post the patches for your compositor?
> > > > > > >
> > > > > > > Also note that we already allow userspace to tell the kernel whether
> > > > > > > flickering is ok or not for a modeset. msm driver could use that to at
> > > > > > > least tell userspace whether a modeset change is possible. So you can
> > > > > > > already implement glitch-free modeset changes for at least video mode.
> > > > > > > -Daniel
> > > > > >
> > > > > > I believe you are referring to the below tv property of the connector.
> > > > > >
> > > > > > /**
> > > > > >  * @tv_flicker_reduction_property: Optional TV property to control the
> > > > > >  * flicker reduction mode.
> > > > > >  */
> > > > > > struct drm_property *tv_flicker_reduction_property;
> > > > >
> > > > > Not even close :-)
> > > > >
> > > > > I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic ioctl. This
> > > > > is not a property of a mode, this is a property of a _transition_ between
> > > > > configurations. Some transitions can be done flicker free, others can't.
> > > >
> > > > Agree that an atomic flag on a commit is the way to accomplish this. It's pretty
> > > > similar to the psr transitions, where we want to reuse most of the atomic
> > > > circuitry, but in a specialized way. We'd also have to be careful to only
> > > > involve the drm objects which are seamless modeset aware (you could imagine
> > > > a bridge chain where the bridges downstream of the first bridge don't care).
> > > >
> > > > >
> > > > > There's then still the question of how to pick video vs command mode, but
> > > > > imo better to start with implementing the transition behaviour correctly
> > > > > first.
> > > >
> > > > Connector property? Possibly a terrible idea, but I wonder if we could [re]use
> > > > the vrr properties for command mode. The docs state that the driver has the
> > > > option of putting upper and lower bounds on the refresh rate.
> > > 
> > > Not really sure why this needs new props and whatnot. This is kinda
> > > what
> > > the i915 "fastset" stuff already does:
> > > 1. userspace asks for something to be changed via atomic
> > > 2. driver calculates whether a modeset is actually required
> > > 3. atomic validates need_modeset() vs. DRM_MODE_ATOMIC_ALLOW_MODESET
> > > 4. if (need_modeset) heavyweight_commit() else lightweight_commit()
> > > 
> > > Ie. why should userspace really care about anything except the
> > > "flickers are OK" vs. "flickers not wanted" thing?
> > 
> > Agree, I don't think the seamless modeset (ie: changing resolution
> > without
> > flicker) needs a property. Just need to test the commit without
> > ALLOW_MODESET
> > and commit it if the test passes.
> > 
> 
> Agreed that a TEST_ONLY commit without ALLOW_MODESET flag can be used to
> check
> whether the modeset can be done seamless. But since there are no error code
> returns,
> the client cannot distinguish the test_only commit failures from other
> invalid config failures.
> 
> Also, note that when the client sees two 1080p modes (vid/cmd) and it is
> interested only
> to do *only* seamless switches, without any additional flag it cannot
> distinguish between
> these two 1080p modes. The client has to invoke two test_only commits with
> these
> modes to identify the seamless one. Is that a preferred approach?

Hi Jey!
Yeah, pretty much. Stepping back a bit though, why is the kernel exposing 2
1080p modes in the first place? If you just expose one mode and then use a
property to enter "low-latency operation" (or overloading vrr for cmd mode), you
shouldn't need to do the mode switch, just flip the property and let the kernel
figure out how to transition to video/cmd mode.

> 
> Intel's "fastset" calculates the need for modeset internally based on the
> configuration and chooses the best commit path. But the requirement here
> is to expose the information up-front since the use case cannot afford
> to fall back to the normal modeset when it has requested for a seamless one.
> 
> > > 
> > > Also what's the benefit of using video mode if your panel supportes
> > > command mode? Can you turn off the memory in the panel and actually
> > > save power that way? And if there is a benefit can't the driver just
> > > automagically switch between the two based on how often things are
> > > getting updated? That would match how eDP PSR already works.
> > 
> > I'm guessing video mode might have some latency benefits over command
> > mode?
> > 
> > Sean
> 
> Yes. Pretty much those are reasons we need to switch to video mode. But
> instead
> of making the decision internal to the driver based on the frequency of
> frame updates,
> we have proprietary use cases where the client has to trigger the switch
> explicitly.

Unsolicited advice: if you find yourself typing "proprietary" while justifying
an upstream proposal, reword your argument immediately :-)

Now that I've filled my awful joke quota for the mail, I can move on. Generally
speaking, it's better to you expand on why userspace needs this. There's a very
good chance that if it makes sense for Qualcomm, it makes sense for others. If
we have an open discussion about it, others might chime in with "me too" and
that will help your case (and you might even hook someone into doing the work
for you!). Alternatively, we might be able to solve the problem in a different
way that makes everyone happy (or most everyone happy).

There's 100% a case for allowing userspace to trigger a low-latency mode at the
expense of power (we should have it disable the psr entry timer as well). Let's
steer this conversation in the direction of "can we use something that already
exists, or should we add something new?"

Sean


> So we are trying to find ways to represent the same resolution in both
> video/cmd modes.
> 
> Thanks and Regards,
> Jeykumar S.
> 
> > 
> > > 
> > > --
> > > Ville Syrjälä
> > > Intel
> 
> -- 
> Jeykumar S

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC] Expanding drm_mode_modeinfo flags

[Jeykumar Sankaran]

On 2019-07-24 07:48, Sean Paul wrote:
> On Mon, Jul 22, 2019 at 04:50:43PM -0700, Jeykumar Sankaran wrote:
>> On 2019-07-19 07:29, Sean Paul wrote:
>> > On Fri, Jul 19, 2019 at 05:15:28PM +0300, Ville Syrjälä wrote:
>> > > On Fri, Jul 19, 2019 at 09:55:58AM -0400, Sean Paul wrote:
>> > > > On Fri, Jul 19, 2019 at 11:05:53AM +0200, Daniel Vetter wrote:
>> > > > > On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran
> wrote:
>> > > > > > On 2019-07-16 02:07, Daniel Vetter wrote:
>> > > > > > > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran
> wrote:
>> >
>> > /snip
>> >
>> > > > > > > >   drm: add mode flags in uapi for seamless mode switch
>> > > > > > >
>> > > > > > > I think the uapi is the trivial part here, the real deal is
> how
>> > > > > > > userspace
>> > > > > > > uses this. Can you pls post the patches for your compositor?
>> > > > > > >
>> > > > > > > Also note that we already allow userspace to tell the kernel
> whether
>> > > > > > > flickering is ok or not for a modeset. msm driver could use
> that to at
>> > > > > > > least tell userspace whether a modeset change is possible.
> So you can
>> > > > > > > already implement glitch-free modeset changes for at least
> video mode.
>> > > > > > > -Daniel
>> > > > > >
>> > > > > > I believe you are referring to the below tv property of the
> connector.
>> > > > > >
>> > > > > > /**
>> > > > > >  * @tv_flicker_reduction_property: Optional TV property to
> control the
>> > > > > >  * flicker reduction mode.
>> > > > > >  */
>> > > > > > struct drm_property *tv_flicker_reduction_property;
>> > > > >
>> > > > > Not even close :-)
>> > > > >
>> > > > > I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic
> ioctl. This
>> > > > > is not a property of a mode, this is a property of a
> _transition_ between
>> > > > > configurations. Some transitions can be done flicker free,
> others can't.
>> > > >
>> > > > Agree that an atomic flag on a commit is the way to accomplish
> this. It's pretty
>> > > > similar to the psr transitions, where we want to reuse most of the
> atomic
>> > > > circuitry, but in a specialized way. We'd also have to be careful
> to only
>> > > > involve the drm objects which are seamless modeset aware (you
> could imagine
>> > > > a bridge chain where the bridges downstream of the first bridge
> don't care).
>> > > >
>> > > > >
>> > > > > There's then still the question of how to pick video vs command
> mode, but
>> > > > > imo better to start with implementing the transition behaviour
> correctly
>> > > > > first.
>> > > >
>> > > > Connector property? Possibly a terrible idea, but I wonder if we
> could [re]use
>> > > > the vrr properties for command mode. The docs state that the
> driver has the
>> > > > option of putting upper and lower bounds on the refresh rate.
>> > >
>> > > Not really sure why this needs new props and whatnot. This is kinda
>> > > what
>> > > the i915 "fastset" stuff already does:
>> > > 1. userspace asks for something to be changed via atomic
>> > > 2. driver calculates whether a modeset is actually required
>> > > 3. atomic validates need_modeset() vs. DRM_MODE_ATOMIC_ALLOW_MODESET
>> > > 4. if (need_modeset) heavyweight_commit() else lightweight_commit()
>> > >
>> > > Ie. why should userspace really care about anything except the
>> > > "flickers are OK" vs. "flickers not wanted" thing?
>> >
>> > Agree, I don't think the seamless modeset (ie: changing resolution
>> > without
>> > flicker) needs a property. Just need to test the commit without
>> > ALLOW_MODESET
>> > and commit it if the test passes.
>> >
>> 
>> Agreed that a TEST_ONLY commit without ALLOW_MODESET flag can be used 
>> to
>> check
>> whether the modeset can be done seamless. But since there are no error
> code
>> returns,
>> the client cannot distinguish the test_only commit failures from other
>> invalid config failures.
>> 
>> Also, note that when the client sees two 1080p modes (vid/cmd) and it 
>> is
>> interested only
>> to do *only* seamless switches, without any additional flag it cannot
>> distinguish between
>> these two 1080p modes. The client has to invoke two test_only commits
> with
>> these
>> modes to identify the seamless one. Is that a preferred approach?
> 
> Hi Jey!
> Yeah, pretty much. Stepping back a bit though, why is the kernel 
> exposing
> 2
> 1080p modes in the first place? If you just expose one mode and then 
> use a
> property to enter "low-latency operation" (or overloading vrr for cmd
> mode), you
> shouldn't need to do the mode switch, just flip the property and let 
> the
> kernel
> figure out how to transition to video/cmd mode.
> 
>> 
>> Intel's "fastset" calculates the need for modeset internally based on
> the
>> configuration and chooses the best commit path. But the requirement 
>> here
>> is to expose the information up-front since the use case cannot afford
>> to fall back to the normal modeset when it has requested for a 
>> seamless
> one.
>> 
>> > >
>> > > Also what's the benefit of using video mode if your panel supportes
>> > > command mode? Can you turn off the memory in the panel and actually
>> > > save power that way? And if there is a benefit can't the driver just
>> > > automagically switch between the two based on how often things are
>> > > getting updated? That would match how eDP PSR already works.
>> >
>> > I'm guessing video mode might have some latency benefits over command
>> > mode?
>> >
>> > Sean
>> 
>> Yes. Pretty much those are reasons we need to switch to video mode. 
>> But
>> instead
>> of making the decision internal to the driver based on the frequency 
>> of
>> frame updates,
>> we have proprietary use cases where the client has to trigger the 
>> switch
>> explicitly.
> 
> Unsolicited advice: if you find yourself typing "proprietary" while
> justifying
> an upstream proposal, reword your argument immediately :-)
> 
> Now that I've filled my awful joke quota for the mail, I can move on.
> Generally
> speaking, it's better to you expand on why userspace needs this. 
> There's a
> very
> good chance that if it makes sense for Qualcomm, it makes sense for
> others. If
> we have an open discussion about it, others might chime in with "me 
> too"
> and
> that will help your case (and you might even hook someone into doing 
> the
> work
> for you!). Alternatively, we might be able to solve the problem in a
> different
> way that makes everyone happy (or most everyone happy).
> 
> There's 100% a case for allowing userspace to trigger a low-latency 
> mode
> at the
> expense of power (we should have it disable the psr entry timer as 
> well).
> Let's
> steer this conversation in the direction of "can we use something that
> already
> exists, or should we add something new?"
> 
> Sean
> 
Thank you Sean for the suggestion! I can use the proposed property 
approach to
switch between vid/cmd mode if every resolution is capable of switching
between these two modes. But we have uses cases where the switch is
restricted only between a set of resolutions.

Also, appreciate your guidance for formating closed usecase questions. 
Let me
check how much more details I can provide on why H/W has to enforce such
restrictions and will revive the thread if needed.

Thanks and Regards,
Jeykumar S.

> 
>> So we are trying to find ways to represent the same resolution in both
>> video/cmd modes.
>> 
>> Thanks and Regards,
>> Jeykumar S.
>> 
>> >
>> > >
>> > > --
>> > > Ville Syrjälä
>> > > Intel
>> 
>> --
>> Jeykumar S
> 
> --
> Sean Paul, Software Engineer, Google / Chromium OS

-- 
Jeykumar S

Re: [RFC] Expanding drm_mode_modeinfo flags

[Jeykumar Sankaran]

On 2019-07-24 07:48, Sean Paul wrote:
> On Mon, Jul 22, 2019 at 04:50:43PM -0700, Jeykumar Sankaran wrote:
>> On 2019-07-19 07:29, Sean Paul wrote:
>> > On Fri, Jul 19, 2019 at 05:15:28PM +0300, Ville Syrjälä wrote:
>> > > On Fri, Jul 19, 2019 at 09:55:58AM -0400, Sean Paul wrote:
>> > > > On Fri, Jul 19, 2019 at 11:05:53AM +0200, Daniel Vetter wrote:
>> > > > > On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran
> wrote:
>> > > > > > On 2019-07-16 02:07, Daniel Vetter wrote:
>> > > > > > > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran
> wrote:
>> >
>> > /snip
>> >
>> > > > > > > >   drm: add mode flags in uapi for seamless mode switch
>> > > > > > >
>> > > > > > > I think the uapi is the trivial part here, the real deal is
> how
>> > > > > > > userspace
>> > > > > > > uses this. Can you pls post the patches for your compositor?
>> > > > > > >
>> > > > > > > Also note that we already allow userspace to tell the kernel
> whether
>> > > > > > > flickering is ok or not for a modeset. msm driver could use
> that to at
>> > > > > > > least tell userspace whether a modeset change is possible.
> So you can
>> > > > > > > already implement glitch-free modeset changes for at least
> video mode.
>> > > > > > > -Daniel
>> > > > > >
>> > > > > > I believe you are referring to the below tv property of the
> connector.
>> > > > > >
>> > > > > > /**
>> > > > > >  * @tv_flicker_reduction_property: Optional TV property to
> control the
>> > > > > >  * flicker reduction mode.
>> > > > > >  */
>> > > > > > struct drm_property *tv_flicker_reduction_property;
>> > > > >
>> > > > > Not even close :-)
>> > > > >
>> > > > > I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic
> ioctl. This
>> > > > > is not a property of a mode, this is a property of a
> _transition_ between
>> > > > > configurations. Some transitions can be done flicker free,
> others can't.
>> > > >
>> > > > Agree that an atomic flag on a commit is the way to accomplish
> this. It's pretty
>> > > > similar to the psr transitions, where we want to reuse most of the
> atomic
>> > > > circuitry, but in a specialized way. We'd also have to be careful
> to only
>> > > > involve the drm objects which are seamless modeset aware (you
> could imagine
>> > > > a bridge chain where the bridges downstream of the first bridge
> don't care).
>> > > >
>> > > > >
>> > > > > There's then still the question of how to pick video vs command
> mode, but
>> > > > > imo better to start with implementing the transition behaviour
> correctly
>> > > > > first.
>> > > >
>> > > > Connector property? Possibly a terrible idea, but I wonder if we
> could [re]use
>> > > > the vrr properties for command mode. The docs state that the
> driver has the
>> > > > option of putting upper and lower bounds on the refresh rate.
>> > >
>> > > Not really sure why this needs new props and whatnot. This is kinda
>> > > what
>> > > the i915 "fastset" stuff already does:
>> > > 1. userspace asks for something to be changed via atomic
>> > > 2. driver calculates whether a modeset is actually required
>> > > 3. atomic validates need_modeset() vs. DRM_MODE_ATOMIC_ALLOW_MODESET
>> > > 4. if (need_modeset) heavyweight_commit() else lightweight_commit()
>> > >
>> > > Ie. why should userspace really care about anything except the
>> > > "flickers are OK" vs. "flickers not wanted" thing?
>> >
>> > Agree, I don't think the seamless modeset (ie: changing resolution
>> > without
>> > flicker) needs a property. Just need to test the commit without
>> > ALLOW_MODESET
>> > and commit it if the test passes.
>> >
>> 
>> Agreed that a TEST_ONLY commit without ALLOW_MODESET flag can be used 
>> to
>> check
>> whether the modeset can be done seamless. But since there are no error
> code
>> returns,
>> the client cannot distinguish the test_only commit failures from other
>> invalid config failures.
>> 
>> Also, note that when the client sees two 1080p modes (vid/cmd) and it 
>> is
>> interested only
>> to do *only* seamless switches, without any additional flag it cannot
>> distinguish between
>> these two 1080p modes. The client has to invoke two test_only commits
> with
>> these
>> modes to identify the seamless one. Is that a preferred approach?
> 
> Hi Jey!
> Yeah, pretty much. Stepping back a bit though, why is the kernel 
> exposing
> 2
> 1080p modes in the first place? If you just expose one mode and then 
> use a
> property to enter "low-latency operation" (or overloading vrr for cmd
> mode), you
> shouldn't need to do the mode switch, just flip the property and let 
> the
> kernel
> figure out how to transition to video/cmd mode.
> 
>> 
>> Intel's "fastset" calculates the need for modeset internally based on
> the
>> configuration and chooses the best commit path. But the requirement 
>> here
>> is to expose the information up-front since the use case cannot afford
>> to fall back to the normal modeset when it has requested for a 
>> seamless
> one.
>> 
>> > >
>> > > Also what's the benefit of using video mode if your panel supportes
>> > > command mode? Can you turn off the memory in the panel and actually
>> > > save power that way? And if there is a benefit can't the driver just
>> > > automagically switch between the two based on how often things are
>> > > getting updated? That would match how eDP PSR already works.
>> >
>> > I'm guessing video mode might have some latency benefits over command
>> > mode?
>> >
>> > Sean
>> 
>> Yes. Pretty much those are reasons we need to switch to video mode. 
>> But
>> instead
>> of making the decision internal to the driver based on the frequency 
>> of
>> frame updates,
>> we have proprietary use cases where the client has to trigger the 
>> switch
>> explicitly.
> 
> Unsolicited advice: if you find yourself typing "proprietary" while
> justifying
> an upstream proposal, reword your argument immediately :-)
> 
> Now that I've filled my awful joke quota for the mail, I can move on.
> Generally
> speaking, it's better to you expand on why userspace needs this. 
> There's a
> very
> good chance that if it makes sense for Qualcomm, it makes sense for
> others. If
> we have an open discussion about it, others might chime in with "me 
> too"
> and
> that will help your case (and you might even hook someone into doing 
> the
> work
> for you!). Alternatively, we might be able to solve the problem in a
> different
> way that makes everyone happy (or most everyone happy).
> 
> There's 100% a case for allowing userspace to trigger a low-latency 
> mode
> at the
> expense of power (we should have it disable the psr entry timer as 
> well).
> Let's
> steer this conversation in the direction of "can we use something that
> already
> exists, or should we add something new?"
> 
> Sean
> 
Thank you Sean for the suggestion! I can use the proposed property 
approach to
switch between vid/cmd mode if every resolution is capable of switching
between these two modes. But we have uses cases where the switch is
restricted only between a set of resolutions.

Also, appreciate your guidance for formating closed usecase questions. 
Let me
check how much more details I can provide on why H/W has to enforce such
restrictions and will revive the thread if needed.

Thanks and Regards,
Jeykumar S.

> 
>> So we are trying to find ways to represent the same resolution in both
>> video/cmd modes.
>> 
>> Thanks and Regards,
>> Jeykumar S.
>> 
>> >
>> > >
>> > > --
>> > > Ville Syrjälä
>> > > Intel
>> 
>> --
>> Jeykumar S
> 
> --
> Sean Paul, Software Engineer, Google / Chromium OS

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno