All of lore.kernel.org
 help / color / mirror / Atom feed
From: Imre Deak <imre.deak@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>, stable@vger.kernel.org
Subject: [Intel-gfx] [PATCH 2/6] drm/i915/dp: Ensure sink rate values are always valid
Date: Mon, 18 Oct 2021 12:41:50 +0300	[thread overview]
Message-ID: <20211018094154.1407705-3-imre.deak@intel.com> (raw)
In-Reply-To: <20211018094154.1407705-1-imre.deak@intel.com>

Atm, there are no sink rate values set for DP (vs. eDP) sinks until the
DPCD capabilities are successfully read from the sink. During this time
intel_dp->num_common_rates is 0 which can lead to a

intel_dp->common_rates[-1]    (*)

access, which is an undefined behaviour, in the following cases:

- In intel_dp_sync_state(), if the encoder is enabled without a sink
  connected to the encoder's connector (BIOS enabled a monitor, but the
  user unplugged the monitor until the driver loaded).
- In intel_dp_sync_state() if the encoder is enabled with a sink
  connected, but for some reason the DPCD read has failed.
- In intel_dp_compute_link_config() if modesetting a connector without
  a sink connected on it.
- In intel_dp_compute_link_config() if modesetting a connector with a
  a sink connected on it, but before probing the connector first.

To avoid the (*) access in all the above cases, make sure that the sink
rate table - and hence the common rate table - is always valid, by
setting a default minimum sink rate when registering the connector
before anything could use it.

I also considered setting all the DP link rates by default, so that
modesetting with higher resolution modes also succeeds in the last two
cases above. However in case a sink is not connected that would stop
working after the first modeset, due to the LT fallback logic. So this
would need more work, beyond the scope of this fix.

As I mentioned in the previous patch, I don't think the issue this patch
fixes is user visible, however it is an undefined behaviour by
definition and triggers a BUG() in CONFIG_UBSAN builds, hence CC:stable.

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4297
References: https://gitlab.freedesktop.org/drm/intel/-/issues/4298
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 23de500d56b52..153ae944a354b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -120,6 +120,12 @@ bool intel_dp_is_uhbr(const struct intel_crtc_state *crtc_state)
 	return crtc_state->port_clock >= 1000000;
 }
 
+static void intel_dp_set_default_sink_rates(struct intel_dp *intel_dp)
+{
+	intel_dp->sink_rates[0] = 162000;
+	intel_dp->num_sink_rates = 1;
+}
+
 /* update sink rates from dpcd */
 static void intel_dp_set_sink_rates(struct intel_dp *intel_dp)
 {
@@ -5003,6 +5009,8 @@ intel_dp_init_connector(struct intel_digital_port *dig_port,
 	}
 
 	intel_dp_set_source_rates(intel_dp);
+	intel_dp_set_default_sink_rates(intel_dp);
+	intel_dp_set_common_rates(intel_dp);
 
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 		intel_dp->pps.active_pipe = vlv_active_pipe(intel_dp);
-- 
2.27.0


WARNING: multiple messages have this Message-ID (diff)
From: Imre Deak <imre.deak@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>, stable@vger.kernel.org
Subject: [PATCH 2/6] drm/i915/dp: Ensure sink rate values are always valid
Date: Mon, 18 Oct 2021 12:41:50 +0300	[thread overview]
Message-ID: <20211018094154.1407705-3-imre.deak@intel.com> (raw)
In-Reply-To: <20211018094154.1407705-1-imre.deak@intel.com>

Atm, there are no sink rate values set for DP (vs. eDP) sinks until the
DPCD capabilities are successfully read from the sink. During this time
intel_dp->num_common_rates is 0 which can lead to a

intel_dp->common_rates[-1]    (*)

access, which is an undefined behaviour, in the following cases:

- In intel_dp_sync_state(), if the encoder is enabled without a sink
  connected to the encoder's connector (BIOS enabled a monitor, but the
  user unplugged the monitor until the driver loaded).
- In intel_dp_sync_state() if the encoder is enabled with a sink
  connected, but for some reason the DPCD read has failed.
- In intel_dp_compute_link_config() if modesetting a connector without
  a sink connected on it.
- In intel_dp_compute_link_config() if modesetting a connector with a
  a sink connected on it, but before probing the connector first.

To avoid the (*) access in all the above cases, make sure that the sink
rate table - and hence the common rate table - is always valid, by
setting a default minimum sink rate when registering the connector
before anything could use it.

I also considered setting all the DP link rates by default, so that
modesetting with higher resolution modes also succeeds in the last two
cases above. However in case a sink is not connected that would stop
working after the first modeset, due to the LT fallback logic. So this
would need more work, beyond the scope of this fix.

As I mentioned in the previous patch, I don't think the issue this patch
fixes is user visible, however it is an undefined behaviour by
definition and triggers a BUG() in CONFIG_UBSAN builds, hence CC:stable.

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4297
References: https://gitlab.freedesktop.org/drm/intel/-/issues/4298
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 23de500d56b52..153ae944a354b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -120,6 +120,12 @@ bool intel_dp_is_uhbr(const struct intel_crtc_state *crtc_state)
 	return crtc_state->port_clock >= 1000000;
 }
 
+static void intel_dp_set_default_sink_rates(struct intel_dp *intel_dp)
+{
+	intel_dp->sink_rates[0] = 162000;
+	intel_dp->num_sink_rates = 1;
+}
+
 /* update sink rates from dpcd */
 static void intel_dp_set_sink_rates(struct intel_dp *intel_dp)
 {
@@ -5003,6 +5009,8 @@ intel_dp_init_connector(struct intel_digital_port *dig_port,
 	}
 
 	intel_dp_set_source_rates(intel_dp);
+	intel_dp_set_default_sink_rates(intel_dp);
+	intel_dp_set_common_rates(intel_dp);
 
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 		intel_dp->pps.active_pipe = vlv_active_pipe(intel_dp);
-- 
2.27.0


  parent reply	other threads:[~2021-10-18  9:42 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-18  9:41 [Intel-gfx] [PATCH 0/6] drm/i915/dp: Fix link parameter use in lack of a valid DPCD Imre Deak
2021-10-18  9:41 ` [Intel-gfx] [PATCH 1/6] drm/i915/dp: Skip the HW readout of DPCD on disabled encoders Imre Deak
2021-10-18  9:41   ` Imre Deak
2021-10-18  9:41 ` Imre Deak [this message]
2021-10-18  9:41   ` [PATCH 2/6] drm/i915/dp: Ensure sink rate values are always valid Imre Deak
2021-10-18 14:34   ` [PATCH v2 " Imre Deak
2021-10-18 14:34     ` [Intel-gfx] " Imre Deak
2021-10-19  7:27   ` [Intel-gfx] [PATCH " Jani Nikula
2021-10-19  7:33     ` Imre Deak
2021-10-19  7:37       ` Jani Nikula
2021-10-19  7:39         ` Imre Deak
2021-10-19 18:37           ` Imre Deak
2021-10-19 19:17             ` Jani Nikula
2021-10-18  9:41 ` [Intel-gfx] [PATCH 3/6] drm/i915/dp: Ensure max link params " Imre Deak
2021-10-18  9:41   ` Imre Deak
2021-10-18  9:41 ` [Intel-gfx] [PATCH 4/6] drm/i915/dp: Ensure sink/link max lane count values " Imre Deak
2021-10-18 15:04   ` Ville Syrjälä
2021-10-18 15:13     ` Imre Deak
2021-10-18 15:27       ` Ville Syrjälä
2021-10-18  9:41 ` [Intel-gfx] [PATCH 5/6] drm/i915/dp: Sanitize sink rate DPCD register values Imre Deak
2021-10-18  9:41 ` [Intel-gfx] [PATCH 6/6] drm/i915/dp: Sanitize link common rate array lookups Imre Deak
2021-10-19 19:23   ` Jani Nikula
2021-10-20  9:06     ` Imre Deak
2021-10-20  9:53       ` Jani Nikula
2021-10-20 10:09         ` Ville Syrjälä
2021-10-18 12:31 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dp: Fix link parameter use in lack of a valid DPCD Patchwork
2021-10-18 12:33 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-10-18 13:06 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2021-10-18 18:01 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dp: Fix link parameter use in lack of a valid DPCD (rev2) Patchwork
2021-10-18 18:03 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-10-18 18:31 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-10-19  0:52 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2021-10-19 12:54   ` Imre Deak
2021-10-19 15:33     ` Vudum, Lakshminarayana
2021-10-19 16:32       ` Imre Deak
2021-10-19 14:45 ` [Intel-gfx] ✓ Fi.CI.IGT: success " Patchwork
2021-10-20 15:40   ` Imre Deak

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=20211018094154.1407705-3-imre.deak@intel.com \
    --to=imre.deak@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=stable@vger.kernel.org \
    --cc=ville.syrjala@linux.intel.com \
    /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.