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

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

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 <lyude at redhat.com>
---
 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.
  *
- *    - &drm_connector_helper_funcs.get_modes vfunc
+ *    - &drm_connector_helper_funcs.get_modes_ctx vfunc if set, otherwise
+ *      &drm_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, &ctx);
+		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 &drm_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 &drm_connector.display_info,
+	 * &drm_connector.width_mm and &drm_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 &drm_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

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 <lyude at redhat.com>
---
 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 &nv50_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, &clock);
+	if (*status != MODE_OK)
+		return 0;
 
-	return nv50_dp_mode_valid(connector, outp, mode, NULL);
+	ret = drm_modeset_lock(&mstc->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(&mstc->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(&mstc->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(&mstc->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 <nvif/event.h>
 
 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, &connector->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,
+						     &status);
+			if (ret < 0)
+				return ERR_PTR(ret);
+			if (status != MODE_OK)
+				continue;
+		} else if (helper->mode_valid(connector, mode) != MODE_OK) {
+			continue;
+		}
+
 		/* Use preferred mode if there is one.. */
 		if (mode->type & DRM_MODE_TYPE_PREFERRED) {
 			NV_DEBUG(drm, "native mode from preferred\n");
@@ -959,7 +974,8 @@ nouveau_connector_get_modes(struct drm_connector *connector)
 	 * the list of modes.
 	 */
 	if (!nv_connector->native_mode)
-		nv_connector->native_mode = nouveau_conn_native_mode(connector);
+		nv_connector->native_mode +			nouveau_conn_native_mode(connector, NULL);
 	if (ret == 0 && nv_connector->native_mode) {
 		struct drm_display_mode *mode;
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h
b/drivers/gpu/drm/nouveau/nouveau_connector.h
index 9e062c7adec8..e85962d9ae66 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.h
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
@@ -194,7 +194,9 @@ int nouveau_conn_atomic_set_property(struct drm_connector *,
 int nouveau_conn_atomic_get_property(struct drm_connector *,
 				     const struct drm_connector_state *,
 				     struct drm_property *, u64 *);
-struct drm_display_mode *nouveau_conn_native_mode(struct drm_connector *);
+struct drm_display_mode *
+nouveau_conn_native_mode(struct drm_connector *connector,
+			 struct drm_modeset_acquire_ctx *ctx);
 enum drm_mode_status
 nouveau_conn_mode_clock_valid(const struct drm_display_mode *,
 			      const unsigned min_clock,
-- 
2.26.2