drm: document how user-space should use link-status
diff mbox series

Message ID krnCwRP0UCcVJbY-8ILP_gEFf4EaUdKPSuuHisFkphFaoOl2EAnU032oOWAeJi2xlsFsA7qeR8lypXs71-SoULZnd2gP5C7ohDEfsWTB5-A=@emersion.fr
State New
Headers show
Series
  • drm: document how user-space should use link-status
Related show

Commit Message

Simon Ser June 1, 2020, 2:48 p.m. UTC
Describe what a "BAD" link-status means for user-space and how it should
handle it. The logic described has been implemented in igt [1].

[1]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/commit/fbe61f529737191d0920521946a575bd55f00fbe

Signed-off-by: Simon Ser <contact@emersion.fr>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
---
 drivers/gpu/drm/drm_connector.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Pekka Paalanen June 2, 2020, 7:38 a.m. UTC | #1
On Mon, 01 Jun 2020 14:48:58 +0000
Simon Ser <contact@emersion.fr> wrote:

> Describe what a "BAD" link-status means for user-space and how it should
> handle it. The logic described has been implemented in igt [1].
> 
> [1]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/commit/fbe61f529737191d0920521946a575bd55f00fbe
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Pekka Paalanen <ppaalanen@gmail.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index f2b20fd66319..08ba84f9787a 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -994,6 +994,12 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
>   *      after modeset, the kernel driver may set this to "BAD" and issue a
>   *      hotplug uevent. Drivers should update this value using
>   *      drm_connector_set_link_status_property().
> + *
> + *      When user-space receives the hotplug uevent and detects a "BAD"
> + *      link-status, the connector is no longer enabled. The list of available
> + *      modes may have changed. User-space is expected to pick a new mode if
> + *      the current one has disappeared and perform a new modeset with
> + *      link-status set to "GOOD" to re-enable the connector.
>   * non_desktop:
>   * 	Indicates the output should be ignored for purposes of displaying a
>   * 	standard desktop environment or console. This is most likely because

Hi,

makes sense to me. Can it happen that there will be no modes left in
the list?

What if userspace is driving two connectors from the same CRTC, and only
one connector gets link-status bad, what does it mean? Is the other
connector still running as normal, as if the failed connector didn't
even exist?

That is mostly a question about what happens if userspace does not fix
up the link-status=bad connector and does not detach it from the CRTC,
but keeps on flipping or modesetting as if the failure never happened.
I guess I could ask it about both a CRTC that has another connector
still good, and a CRTC where the failed connector was the only one.

Can I trust that if the other connector is in any way affected, it too
will get link-status bad?


Thanks,
pq
Daniel Vetter June 2, 2020, 9:58 a.m. UTC | #2
On Tue, Jun 02, 2020 at 10:38:46AM +0300, Pekka Paalanen wrote:
> On Mon, 01 Jun 2020 14:48:58 +0000
> Simon Ser <contact@emersion.fr> wrote:
> 
> > Describe what a "BAD" link-status means for user-space and how it should
> > handle it. The logic described has been implemented in igt [1].
> > 
> > [1]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/commit/fbe61f529737191d0920521946a575bd55f00fbe
> > 
> > Signed-off-by: Simon Ser <contact@emersion.fr>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Cc: Pekka Paalanen <ppaalanen@gmail.com>
> > ---
> >  drivers/gpu/drm/drm_connector.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index f2b20fd66319..08ba84f9787a 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -994,6 +994,12 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
> >   *      after modeset, the kernel driver may set this to "BAD" and issue a
> >   *      hotplug uevent. Drivers should update this value using
> >   *      drm_connector_set_link_status_property().
> > + *
> > + *      When user-space receives the hotplug uevent and detects a "BAD"
> > + *      link-status, the connector is no longer enabled. The list of available

"no longer enabled" is kinda wrong, you can keep doing pageflip. It's just
that the pixels aren't getting to the screen anymore.

"enabled" wrt an output has a different meaning in kms, the internal
drm_crtc_state->enabled state is very much still set. Including what that
all means for the uapi.

Also I think we need some words here on what automatically happens when
you do an atomic commit with the connector with the bad link status
(auto-reset to good, which might make the atomic modeset fail). On irc we
had some discussions that we should only do this if ALLOW_MODESET is set,
but that's atm not the case.
-Daniel

> > + *      modes may have changed. User-space is expected to pick a new mode if
> > + *      the current one has disappeared and perform a new modeset with
> > + *      link-status set to "GOOD" to re-enable the connector.
> >   * non_desktop:
> >   * 	Indicates the output should be ignored for purposes of displaying a
> >   * 	standard desktop environment or console. This is most likely because
> 
> Hi,
> 
> makes sense to me. Can it happen that there will be no modes left in
> the list?
> 
> What if userspace is driving two connectors from the same CRTC, and only
> one connector gets link-status bad, what does it mean? Is the other
> connector still running as normal, as if the failed connector didn't
> even exist?
> 
> That is mostly a question about what happens if userspace does not fix
> up the link-status=bad connector and does not detach it from the CRTC,
> but keeps on flipping or modesetting as if the failure never happened.
> I guess I could ask it about both a CRTC that has another connector
> still good, and a CRTC where the failed connector was the only one.
> 
> Can I trust that if the other connector is in any way affected, it too
> will get link-status bad?
> 
> 
> Thanks,
> pq
Simon Ser June 2, 2020, 7:15 p.m. UTC | #3
On Tuesday, June 2, 2020 9:38 AM, Pekka Paalanen <ppaalanen@gmail.com> wrote:

> Can it happen that there will be no modes left in
> the list?

Reading drm_helper_probe_single_connector_modes, this sounds unlikely
but possible.

This isn't specific to link-status though. This can be the case on a
regular hotplug uevent too.

> What if userspace is driving two connectors from the same CRTC, and only
> one connector gets link-status bad, what does it mean? Is the other
> connector still running as normal, as if the failed connector didn't
> even exist?
>
> That is mostly a question about what happens if userspace does not fix
> up the link-status=bad connector and does not detach it from the CRTC,
> but keeps on flipping or modesetting as if the failure never happened.
> I guess I could ask it about both a CRTC that has another connector
> still good, and a CRTC where the failed connector was the only one.
>
> Can I trust that if the other connector is in any way affected, it too
> will get link-status bad?

link-status is about link maintenance (e.g. DP link training), so I
think the other connector would be fine in this case. I'll add this to
the next version and let Daniel/Manasi comment if that's incorrect.
Manasi Navare June 2, 2020, 7:33 p.m. UTC | #4
On Tue, Jun 02, 2020 at 11:58:36AM +0200, Daniel Vetter wrote:
> On Tue, Jun 02, 2020 at 10:38:46AM +0300, Pekka Paalanen wrote:
> > On Mon, 01 Jun 2020 14:48:58 +0000
> > Simon Ser <contact@emersion.fr> wrote:
> > 
> > > Describe what a "BAD" link-status means for user-space and how it should
> > > handle it. The logic described has been implemented in igt [1].
> > > 
> > > [1]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/commit/fbe61f529737191d0920521946a575bd55f00fbe
> > > 
> > > Signed-off-by: Simon Ser <contact@emersion.fr>
> > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > Cc: Pekka Paalanen <ppaalanen@gmail.com>
> > > ---
> > >  drivers/gpu/drm/drm_connector.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > index f2b20fd66319..08ba84f9787a 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -994,6 +994,12 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
> > >   *      after modeset, the kernel driver may set this to "BAD" and issue a
> > >   *      hotplug uevent. Drivers should update this value using
> > >   *      drm_connector_set_link_status_property().
> > > + *
> > > + *      When user-space receives the hotplug uevent and detects a "BAD"
> > > + *      link-status, the connector is no longer enabled. The list of available
> 
> "no longer enabled" is kinda wrong, you can keep doing pageflip. It's just
> that the pixels aren't getting to the screen anymore.
> 
> "enabled" wrt an output has a different meaning in kms, the internal
> drm_crtc_state->enabled state is very much still set. Including what that
> all means for the uapi.

Yes I was about to comment on that too. And here we should mention, that rather
when user space recieves the hotplug uevent and detects a BAD link status, that
means connector's physical link failed to train correctly hence no output across
the link and a black screen seen on the display. In this case the userspace
should respond to this uevent by reprobing the connector to get a modelist
now as per the new capabilities of the connector after the fallback in link rate/lane count
and retry a full modeset resetting the link-status to GOOD

Also to answer Pekka's concern here about modelist being empty, this should not happen
since the kernel fallsback the link capabilities until it reaches the lowest RBR and 1 lane count
and with this most panels should be still able to do the lowest available mode in their modelist.
And likely the link training will pass for this minimum resolution and minimum capabilities.

Manasi

> 
> Also I think we need some words here on what automatically happens when
> you do an atomic commit with the connector with the bad link status
> (auto-reset to good, which might make the atomic modeset fail). On irc we
> had some discussions that we should only do this if ALLOW_MODESET is set,
> but that's atm not the case.
> -Daniel
> 
> > > + *      modes may have changed. User-space is expected to pick a new mode if
> > > + *      the current one has disappeared and perform a new modeset with
> > > + *      link-status set to "GOOD" to re-enable the connector.
> > >   * non_desktop:
> > >   * 	Indicates the output should be ignored for purposes of displaying a
> > >   * 	standard desktop environment or console. This is most likely because
> > 
> > Hi,
> > 
> > makes sense to me. Can it happen that there will be no modes left in
> > the list?
> > 
> > What if userspace is driving two connectors from the same CRTC, and only
> > one connector gets link-status bad, what does it mean? Is the other
> > connector still running as normal, as if the failed connector didn't
> > even exist?
> > 
> > That is mostly a question about what happens if userspace does not fix
> > up the link-status=bad connector and does not detach it from the CRTC,
> > but keeps on flipping or modesetting as if the failure never happened.
> > I guess I could ask it about both a CRTC that has another connector
> > still good, and a CRTC where the failed connector was the only one.
> > 
> > Can I trust that if the other connector is in any way affected, it too
> > will get link-status bad?
> > 
> > 
> > Thanks,
> > pq
> 
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

Patch
diff mbox series

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index f2b20fd66319..08ba84f9787a 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -994,6 +994,12 @@  static const struct drm_prop_enum_list dp_colorspaces[] = {
  *      after modeset, the kernel driver may set this to "BAD" and issue a
  *      hotplug uevent. Drivers should update this value using
  *      drm_connector_set_link_status_property().
+ *
+ *      When user-space receives the hotplug uevent and detects a "BAD"
+ *      link-status, the connector is no longer enabled. The list of available
+ *      modes may have changed. User-space is expected to pick a new mode if
+ *      the current one has disappeared and perform a new modeset with
+ *      link-status set to "GOOD" to re-enable the connector.
  * non_desktop:
  * 	Indicates the output should be ignored for purposes of displaying a
  * 	standard desktop environment or console. This is most likely because