All of lore.kernel.org
 help / color / mirror / Atom feed
From: Karol Herbst <kherbst@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: nouveau@lists.freedesktop.org, stable@vger.kernel.org,
	dri-devel@lists.freedesktop.org, Ben Skeggs <bskeggs@redhat.com>
Subject: [Nouveau] [PATCH 3/3] drm/nouveau/disp: verify mode on atomic_check
Date: Wed, 28 Jun 2023 23:22:48 +0200	[thread overview]
Message-ID: <20230628212248.3798605-3-kherbst@redhat.com> (raw)
In-Reply-To: <20230628212248.3798605-1-kherbst@redhat.com>

We have to verify the selected mode as Userspace might request one which
we can't configure the GPU for.

X with the modesetting DDX is adding a bunch of modes, some so far outside
of hardware limits that things simply break.

Sadly we can't fix X this way as on start it sticks to one mode and
ignores any error and there is really nothing we can do about this, but at
least this way we won't let the GPU run into any errors caused by a non
supported display mode.

However this does prevent X from switching to such a mode, which to be
fair is an improvement as well.

Seen on one of my Tesla GPUs with a connected 4K display.

Link: https://gitlab.freedesktop.org/drm/nouveau/-/issues/199
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Lyude Paul <lyude@redhat.com>
Cc: stable@vger.kernel.org # v6.1+
Signed-off-by: Karol Herbst <kherbst@redhat.com>
---
 drivers/gpu/drm/nouveau/nouveau_connector.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 22c42a5e184d..edf490c1490c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1114,6 +1114,25 @@ nouveau_connector_atomic_check(struct drm_connector *connector, struct drm_atomi
 	struct drm_connector_state *conn_state =
 		drm_atomic_get_new_connector_state(state, connector);
 
+	/* As we can get any random mode from Userspace, we have to make sure the to be set mode
+	 * is valid and does not violate hardware constraints as we rely on it being sane.
+	 */
+	if (conn_state->crtc) {
+		struct drm_crtc_state *crtc_state =
+			drm_atomic_get_crtc_state(state, conn_state->crtc);
+
+		if (IS_ERR(crtc_state))
+			return PTR_ERR(crtc_state);
+
+		if (crtc_state->enable && (crtc_state->mode_changed ||
+					   crtc_state->connectors_changed)) {
+			struct drm_display_mode *mode = &crtc_state->mode;
+
+			if (connector->helper_private->mode_valid(connector, mode) != MODE_OK)
+				return -EINVAL;
+		}
+	}
+
 	if (!nv_conn->dp_encoder || !nv50_has_mst(nouveau_drm(connector->dev)))
 		return 0;
 
-- 
2.41.0


WARNING: multiple messages have this Message-ID (diff)
From: Karol Herbst <kherbst@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: Karol Herbst <kherbst@redhat.com>,
	nouveau@lists.freedesktop.org, stable@vger.kernel.org,
	dri-devel@lists.freedesktop.org, Ben Skeggs <bskeggs@redhat.com>
Subject: [PATCH 3/3] drm/nouveau/disp: verify mode on atomic_check
Date: Wed, 28 Jun 2023 23:22:48 +0200	[thread overview]
Message-ID: <20230628212248.3798605-3-kherbst@redhat.com> (raw)
In-Reply-To: <20230628212248.3798605-1-kherbst@redhat.com>

We have to verify the selected mode as Userspace might request one which
we can't configure the GPU for.

X with the modesetting DDX is adding a bunch of modes, some so far outside
of hardware limits that things simply break.

Sadly we can't fix X this way as on start it sticks to one mode and
ignores any error and there is really nothing we can do about this, but at
least this way we won't let the GPU run into any errors caused by a non
supported display mode.

However this does prevent X from switching to such a mode, which to be
fair is an improvement as well.

Seen on one of my Tesla GPUs with a connected 4K display.

Link: https://gitlab.freedesktop.org/drm/nouveau/-/issues/199
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Lyude Paul <lyude@redhat.com>
Cc: stable@vger.kernel.org # v6.1+
Signed-off-by: Karol Herbst <kherbst@redhat.com>
---
 drivers/gpu/drm/nouveau/nouveau_connector.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 22c42a5e184d..edf490c1490c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1114,6 +1114,25 @@ nouveau_connector_atomic_check(struct drm_connector *connector, struct drm_atomi
 	struct drm_connector_state *conn_state =
 		drm_atomic_get_new_connector_state(state, connector);
 
+	/* As we can get any random mode from Userspace, we have to make sure the to be set mode
+	 * is valid and does not violate hardware constraints as we rely on it being sane.
+	 */
+	if (conn_state->crtc) {
+		struct drm_crtc_state *crtc_state =
+			drm_atomic_get_crtc_state(state, conn_state->crtc);
+
+		if (IS_ERR(crtc_state))
+			return PTR_ERR(crtc_state);
+
+		if (crtc_state->enable && (crtc_state->mode_changed ||
+					   crtc_state->connectors_changed)) {
+			struct drm_display_mode *mode = &crtc_state->mode;
+
+			if (connector->helper_private->mode_valid(connector, mode) != MODE_OK)
+				return -EINVAL;
+		}
+	}
+
 	if (!nv_conn->dp_encoder || !nv50_has_mst(nouveau_drm(connector->dev)))
 		return 0;
 
-- 
2.41.0


WARNING: multiple messages have this Message-ID (diff)
From: Karol Herbst <kherbst@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	Karol Herbst <kherbst@redhat.com>,
	Ben Skeggs <bskeggs@redhat.com>, Lyude Paul <lyude@redhat.com>,
	stable@vger.kernel.org
Subject: [PATCH 3/3] drm/nouveau/disp: verify mode on atomic_check
Date: Wed, 28 Jun 2023 23:22:48 +0200	[thread overview]
Message-ID: <20230628212248.3798605-3-kherbst@redhat.com> (raw)
In-Reply-To: <20230628212248.3798605-1-kherbst@redhat.com>

We have to verify the selected mode as Userspace might request one which
we can't configure the GPU for.

X with the modesetting DDX is adding a bunch of modes, some so far outside
of hardware limits that things simply break.

Sadly we can't fix X this way as on start it sticks to one mode and
ignores any error and there is really nothing we can do about this, but at
least this way we won't let the GPU run into any errors caused by a non
supported display mode.

However this does prevent X from switching to such a mode, which to be
fair is an improvement as well.

Seen on one of my Tesla GPUs with a connected 4K display.

Link: https://gitlab.freedesktop.org/drm/nouveau/-/issues/199
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Lyude Paul <lyude@redhat.com>
Cc: stable@vger.kernel.org # v6.1+
Signed-off-by: Karol Herbst <kherbst@redhat.com>
---
 drivers/gpu/drm/nouveau/nouveau_connector.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 22c42a5e184d..edf490c1490c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1114,6 +1114,25 @@ nouveau_connector_atomic_check(struct drm_connector *connector, struct drm_atomi
 	struct drm_connector_state *conn_state =
 		drm_atomic_get_new_connector_state(state, connector);
 
+	/* As we can get any random mode from Userspace, we have to make sure the to be set mode
+	 * is valid and does not violate hardware constraints as we rely on it being sane.
+	 */
+	if (conn_state->crtc) {
+		struct drm_crtc_state *crtc_state =
+			drm_atomic_get_crtc_state(state, conn_state->crtc);
+
+		if (IS_ERR(crtc_state))
+			return PTR_ERR(crtc_state);
+
+		if (crtc_state->enable && (crtc_state->mode_changed ||
+					   crtc_state->connectors_changed)) {
+			struct drm_display_mode *mode = &crtc_state->mode;
+
+			if (connector->helper_private->mode_valid(connector, mode) != MODE_OK)
+				return -EINVAL;
+		}
+	}
+
 	if (!nv_conn->dp_encoder || !nv50_has_mst(nouveau_drm(connector->dev)))
 		return 0;
 
-- 
2.41.0


  parent reply	other threads:[~2023-06-28 21:23 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-28 21:22 [Nouveau] [PATCH 1/3] drm/nouveau/disp: fix HDMI on gt215+ Karol Herbst
2023-06-28 21:22 ` Karol Herbst
2023-06-28 21:22 ` Karol Herbst
2023-06-28 21:22 ` [Nouveau] [PATCH 2/3] drm/nouveau/disp: drop unused argument in nv50_dp_mode_valid Karol Herbst
2023-06-28 21:22   ` Karol Herbst
2023-06-28 21:22   ` Karol Herbst
2023-06-28 21:22 ` Karol Herbst [this message]
2023-06-28 21:22   ` [PATCH 3/3] drm/nouveau/disp: verify mode on atomic_check Karol Herbst
2023-06-28 21:22   ` Karol Herbst

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230628212248.3798605-3-kherbst@redhat.com \
    --to=kherbst@redhat.com \
    --cc=bskeggs@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nouveau@lists.freedesktop.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.