[Nouveau] [PATCH 2/2] drm/nouveau/kms/nv50-: Check MST display modes against available PBN

2020-07-24 Thread Lyude Paul
While we already check whether a given atomic state can fit in the
available bandwidth for an MST topology, we don't currently bother to
prune display modes on MST connectors if said modes are impossible to
fit on the MST link irregardless of the current display state.

So, let's avoid reporting impossible-to-set modes to userspace by
validating the minimum bandwidth requirements of each display mode
against the maximum bandwidth supported by the MST connector.

Signed-off-by: Lyude Paul 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 41 +++--
 drivers/gpu/drm/nouveau/nouveau_connector.c | 24 ++--
 drivers/gpu/drm/nouveau/nouveau_connector.h |  4 +-
 3 files changed, 53 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 4c2894d8e15b..7b8531edafa5 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1056,24 +1056,38 @@ nv50_mstc_atomic_best_encoder(struct drm_connector 
*connector,
return _head(crtc)->msto->encoder;
 }
 
-static enum drm_mode_status
-nv50_mstc_mode_valid(struct drm_connector *connector,
-struct drm_display_mode *mode)
+static int
+nv50_mstc_mode_valid_ctx(struct drm_connector *connector,
+struct drm_display_mode *mode,
+struct drm_modeset_acquire_ctx *ctx,
+enum drm_mode_status *status)
 {
struct nv50_mstc *mstc = nv50_mstc(connector);
struct nouveau_encoder *outp = mstc->mstm->outp;
+   unsigned int clock;
+   int ret;
 
-   /* TODO: calculate the PBN from the dotclock and validate against the
-* MSTB's max possible PBN
-*/
+   *status = nv50_dp_mode_valid(connector, outp, mode, );
+   if (*status != MODE_OK)
+   return 0;
 
-   return nv50_dp_mode_valid(connector, outp, mode, NULL);
+   ret = drm_modeset_lock(>mstm->mgr.base.lock, ctx);
+   if (ret)
+   return ret;
+
+   if (drm_dp_calc_pbn_mode(clock, connector->display_info.bpc,
+false) > mstc->port->full_pbn)
+   *status = MODE_CLOCK_HIGH;
+
+   return 0;
 }
 
 static int
-nv50_mstc_get_modes(struct drm_connector *connector)
+nv50_mstc_get_modes_ctx(struct drm_connector *connector,
+   struct drm_modeset_acquire_ctx *ctx)
 {
struct nv50_mstc *mstc = nv50_mstc(connector);
+   struct drm_display_mode *mode;
int ret = 0;
 
mstc->edid = drm_dp_mst_get_edid(>connector, mstc->port->mgr, 
mstc->port);
@@ -1093,9 +1107,14 @@ nv50_mstc_get_modes(struct drm_connector *connector)
else
connector->display_info.bpc = 8;
 
+   mode = nouveau_conn_native_mode(>connector, ctx);
+   if (IS_ERR(mode))
+   return PTR_ERR(mode);
+
if (mstc->native)
drm_mode_destroy(mstc->connector.dev, mstc->native);
-   mstc->native = nouveau_conn_native_mode(>connector);
+   mstc->native = mode;
+
return ret;
 }
 
@@ -1156,8 +1175,8 @@ nv50_mstc_detect(struct drm_connector *connector,
 
 static const struct drm_connector_helper_funcs
 nv50_mstc_help = {
-   .get_modes = nv50_mstc_get_modes,
-   .mode_valid = nv50_mstc_mode_valid,
+   .get_modes_ctx = nv50_mstc_get_modes_ctx,
+   .mode_valid_ctx = nv50_mstc_mode_valid_ctx,
.atomic_best_encoder = nv50_mstc_atomic_best_encoder,
.atomic_check = nv50_mstc_atomic_check,
.detect_ctx = nv50_mstc_detect,
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c 
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index ab2c2b2cab10..9737c65e5627 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -51,7 +51,8 @@
 #include 
 
 struct drm_display_mode *
-nouveau_conn_native_mode(struct drm_connector *connector)
+nouveau_conn_native_mode(struct drm_connector *connector,
+struct drm_modeset_acquire_ctx *ctx)
 {
const struct drm_connector_helper_funcs *helper = 
connector->helper_private;
struct nouveau_drm *drm = nouveau_drm(connector->dev);
@@ -60,10 +61,24 @@ nouveau_conn_native_mode(struct drm_connector *connector)
int high_w = 0, high_h = 0, high_v = 0;
 
list_for_each_entry(mode, >probed_modes, head) {
-   if (helper->mode_valid(connector, mode) != MODE_OK ||
-   (mode->flags & DRM_MODE_FLAG_INTERLACE))
+   if (mode->flags & DRM_MODE_FLAG_INTERLACE)
continue;
 
+   if (helper->mode_valid_ctx) {
+   enum drm_mode_status status;
+   int ret;
+
+   drm_WARN_ON(dev, !ctx);
+   ret = helper->mode_valid_ctx(connector, mode, ctx,
+);
+   if (ret 

[Nouveau] [PATCH 1/2] drm/probe_helper: Add drm_connector_helper_funcs->get_modes_ctx()

2020-07-24 Thread Lyude Paul
Currently nouveau relies on being able to use the
drm_connector_helper_funcs->mode_valid() hook within
drm_connector_helper_funcs->get_modes() so that we can ignore invalid
modes when figuring out the connector's native mode. Since we're about
to add a ->mode_valid_ctx() hook for MST, we'll also need to be able to
pass down the current drm acquisition context for use in ->get_modes().
So, add another version of ->get_modes() which accepts an acquisition
context, ->get_modes_ctx().

Signed-off-by: Lyude Paul 
---
 drivers/gpu/drm/drm_probe_helper.c   | 11 +--
 include/drm/drm_modeset_helper_vtables.h | 42 
 2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c 
b/drivers/gpu/drm/drm_probe_helper.c
index d6017726cc2a..fbda6e527b4b 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -377,7 +377,8 @@ EXPORT_SYMBOL(drm_helper_probe_detect);
  *drm_mode_probed_add(). New modes start their life with status as OK.
  *Modes are added from a single source using the following priority order.
  *
- *- _connector_helper_funcs.get_modes vfunc
+ *- _connector_helper_funcs.get_modes_ctx vfunc if set, otherwise
+ *  _connector_helper_funcs.get_modes is used
  *- if the connector status is connector_status_connected, standard
  *  VESA DMT modes up to 1024x768 are automatically added
  *  (drm_add_modes_noedid())
@@ -506,7 +507,13 @@ int drm_helper_probe_single_connector_modes(struct 
drm_connector *connector,
goto prune;
}
 
-   count = (*connector_funcs->get_modes)(connector);
+   if (connector_funcs->get_modes_ctx) {
+   count = connector_funcs->get_modes_ctx(connector, );
+   if (count < 0)
+   goto retry;
+   } else {
+   count = connector_funcs->get_modes(connector);
+   }
 
/*
 * Fallback for when DDC probe failed in drm_get_edid() and thus skipped
diff --git a/include/drm/drm_modeset_helper_vtables.h 
b/include/drm/drm_modeset_helper_vtables.h
index 4efec30f8bad..c1eb96404bcf 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -904,6 +904,48 @@ struct drm_connector_helper_funcs {
 */
int (*get_modes)(struct drm_connector *connector);
 
+   /**
+* @get_modes_ctx:
+*
+* This function should fill in all modes currently valid for the sink
+* into the _connector.probed_modes list. It should also update the
+* EDID property by calling drm_connector_update_edid_property().
+*
+* The usual way to implement this is to cache the EDID retrieved in the
+* probe callback somewhere in the driver-private connector structure.
+* In this function drivers then parse the modes in the EDID and add
+* them by calling drm_add_edid_modes(). But connectors that driver a
+* fixed panel can also manually add specific modes using
+* drm_mode_probed_add(). Drivers which manually add modes should also
+* make sure that the _connector.display_info,
+* _connector.width_mm and _connector.height_mm fields are
+* filled in.
+*
+* Virtual drivers that just want some standard VESA mode with a given
+* resolution can call drm_add_modes_noedid(), and mark the preferred
+* one using drm_set_preferred_mode().
+*
+* This function is only called after the @detect hook has indicated
+* that a sink is connected and when the EDID isn't overridden through
+* sysfs or the kernel commandline.
+*
+* This callback is used by the probe helpers in e.g.
+* drm_helper_probe_single_connector_modes().
+*
+* To allow for accessing the atomic state of modesetting objects, the
+* helper libraries always call this with ctx set to a valid context,
+* and _mode_config.connection_mutex will always be locked with
+* the ctx parameter set to @ctx. This allows for taking additional
+* locks as required.
+*
+* Returns:
+*
+* The number of modes added by calling drm_mode_probed_add(), or a
+* negative error code on failure.
+*/
+   int (*get_modes_ctx)(struct drm_connector *connector,
+struct drm_modeset_acquire_ctx *ctx);
+
/**
 * @detect_ctx:
 *
-- 
2.26.2

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


[Nouveau] [PATCH 0/2] drm/probe_helper, drm/nouveau: Validate MST modes against PBN

2020-07-24 Thread Lyude Paul
Now that we've added the hooks that we've needed for this and used them
in i915, let's add one more hook (which I could use some feedback on,
I'm not sure if it's worth maybe just reworking how we do mode pruning
in nouveau instead...) and start using this in our mst ->mode_valid
callback to filter out impossible to set modes on MST connectors.

Lyude Paul (2):
  drm/probe_helper: Add drm_connector_helper_funcs->get_modes_ctx()
  drm/nouveau/kms/nv50-: Check MST display modes against available PBN

 drivers/gpu/drm/drm_probe_helper.c  | 11 +-
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 41 ++--
 drivers/gpu/drm/nouveau/nouveau_connector.c | 24 ++--
 drivers/gpu/drm/nouveau/nouveau_connector.h |  4 +-
 include/drm/drm_modeset_helper_vtables.h| 42 +
 5 files changed, 104 insertions(+), 18 deletions(-)

-- 
2.26.2

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] nouveau regression with 5.7 caused by "PCI/PM: Assume ports without DLL Link Active train links in 100 ms"

2020-07-24 Thread Bjorn Helgaas
On Fri, Jul 24, 2020 at 12:57:51PM +0300, Mika Westerberg wrote:
> On Thu, Jul 23, 2020 at 10:30:58PM +0200, Karol Herbst wrote:
> > On Wed, Jul 22, 2020 at 11:25 AM Mika Westerberg
> >  wrote:
> > >
> > > On Tue, Jul 21, 2020 at 01:37:12PM -0500, Patrick Volkerding wrote:
> > > > On 7/21/20 10:27 AM, Mika Westerberg wrote:
> > > > > On Tue, Jul 21, 2020 at 11:01:55AM -0400, Lyude Paul wrote:
> > > > >> Sure thing. Also, feel free to let me know if you'd like access to 
> > > > >> one of the
> > > > >> systems we saw breaking with this patch - I'm fairly sure I've got 
> > > > >> one of them
> > > > >> locally at my apartment and don't mind setting up AMT/KVM/SSH
> > > > > Probably no need for remote access (thanks for the offer, though). I
> > > > > attached a test patch to the bug report:
> > > > >
> > > > >   https://bugzilla.kernel.org/show_bug.cgi?id=208597
> > > > >
> > > > > that tries to work it around (based on the ->pm_cap == 0). I wonder if
> > > > > anyone would have time to try it out.
> > > >
> > > >
> > > > Hi Mika,
> > > >
> > > > I can confirm that this patch applied to 5.4.52 fixes the issue with
> > > > hybrid graphics on the Thinkpad X1 Extreme gen2.
> > >
> > > Great, thanks for testing!
> > 
> > yeah, works on the P1G2 as well.
> 
> Thanks for testing!
> 
> Since we have the revert queued for this release cycle, I think I will
> send an updated version of "PCI/PM: Assume ports without DLL Link Active
> train links in 100 ms" after v5.9-rc1 is released that has this
> workaround in place.
> 
> (I'm continuing my vacation so will be offline next week).

Sounds fine, sorry for interrupting your vacation!
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] nouveau regression with 5.7 caused by "PCI/PM: Assume ports without DLL Link Active train links in 100 ms"

2020-07-24 Thread Mika Westerberg
On Thu, Jul 23, 2020 at 10:30:58PM +0200, Karol Herbst wrote:
> On Wed, Jul 22, 2020 at 11:25 AM Mika Westerberg
>  wrote:
> >
> > On Tue, Jul 21, 2020 at 01:37:12PM -0500, Patrick Volkerding wrote:
> > > On 7/21/20 10:27 AM, Mika Westerberg wrote:
> > > > On Tue, Jul 21, 2020 at 11:01:55AM -0400, Lyude Paul wrote:
> > > >> Sure thing. Also, feel free to let me know if you'd like access to one 
> > > >> of the
> > > >> systems we saw breaking with this patch - I'm fairly sure I've got one 
> > > >> of them
> > > >> locally at my apartment and don't mind setting up AMT/KVM/SSH
> > > > Probably no need for remote access (thanks for the offer, though). I
> > > > attached a test patch to the bug report:
> > > >
> > > >   https://bugzilla.kernel.org/show_bug.cgi?id=208597
> > > >
> > > > that tries to work it around (based on the ->pm_cap == 0). I wonder if
> > > > anyone would have time to try it out.
> > >
> > >
> > > Hi Mika,
> > >
> > > I can confirm that this patch applied to 5.4.52 fixes the issue with
> > > hybrid graphics on the Thinkpad X1 Extreme gen2.
> >
> > Great, thanks for testing!
> >
> 
> yeah, works on the P1G2 as well.

Thanks for testing!

Since we have the revert queued for this release cycle, I think I will
send an updated version of "PCI/PM: Assume ports without DLL Link Active
train links in 100 ms" after v5.9-rc1 is released that has this
workaround in place.

(I'm continuing my vacation so will be offline next week).
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau