[Why] Commit: - commit 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload allocation/removement") accidently overwrite the commit - commit 54d217406afe ("drm: use mgr->dev in drm_dbg_kms in drm_dp_add_payload_part2") which cause regression.
[How] Recover the original NULL fix and remove the unnecessary input parameter 'state' for drm_dp_add_payload_part2().
Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload allocation/removement") Reported-by: Leon Weiß leon.weiss@ruhr-uni-bochum.de Link: https://lore.kernel.org/r/38c253ea42072cc825dc969ac4e6b9b600371cc8.camel@ruh... Cc: lyude@redhat.com Cc: imre.deak@intel.com Cc: stable@vger.kernel.org Cc: regressions@lists.linux.dev Signed-off-by: Wayne Lin Wayne.Lin@amd.com --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +- drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- include/drm/display/drm_dp_mst_helper.h | 1 - 5 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index c27063305a13..2c36f3d00ca2 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -363,7 +363,7 @@ void dm_helpers_dp_mst_send_payload_allocation( mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state); new_payload = drm_atomic_get_mst_payload_state(mst_state, aconnector->mst_output_port);
- ret = drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, new_payload); + ret = drm_dp_add_payload_part2(mst_mgr, new_payload);
if (ret) { amdgpu_dm_set_mst_status(&aconnector->mst_status, diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c index 03d528209426..95fd18f24e94 100644 --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c @@ -3421,7 +3421,6 @@ EXPORT_SYMBOL(drm_dp_remove_payload_part2); /** * drm_dp_add_payload_part2() - Execute payload update part 2 * @mgr: Manager to use. - * @state: The global atomic state * @payload: The payload to update * * If @payload was successfully assigned a starting time slot by drm_dp_add_payload_part1(), this @@ -3430,14 +3429,13 @@ EXPORT_SYMBOL(drm_dp_remove_payload_part2); * Returns: 0 on success, negative error code on failure. */ int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr, - struct drm_atomic_state *state, struct drm_dp_mst_atomic_payload *payload) { int ret = 0;
/* Skip failed payloads */ if (payload->payload_allocation_status != DRM_DP_MST_PAYLOAD_ALLOCATION_DFP) { - drm_dbg_kms(state->dev, "Part 1 of payload creation for %s failed, skipping part 2\n", + drm_dbg_kms(mgr->dev, "Part 1 of payload creation for %s failed, skipping part 2\n", payload->port->connector->name); return -EIO; } diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 53aec023ce92..2fba66aec038 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -1160,7 +1160,7 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state, if (first_mst_stream) intel_ddi_wait_for_fec_status(encoder, pipe_config, true);
- drm_dp_add_payload_part2(&intel_dp->mst_mgr, &state->base, + drm_dp_add_payload_part2(&intel_dp->mst_mgr, drm_atomic_get_mst_payload_state(mst_state, connector->port));
if (DISPLAY_VER(dev_priv) >= 12) diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 0c3d88ad0b0e..88728a0b2c25 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -915,7 +915,7 @@ nv50_msto_cleanup(struct drm_atomic_state *state, msto->disabled = false; drm_dp_remove_payload_part2(mgr, new_mst_state, old_payload, new_payload); } else if (msto->enabled) { - drm_dp_add_payload_part2(mgr, state, new_payload); + drm_dp_add_payload_part2(mgr, new_payload); msto->enabled = false; } } diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h index 9b19d8bd520a..6c9145abc7e2 100644 --- a/include/drm/display/drm_dp_mst_helper.h +++ b/include/drm/display/drm_dp_mst_helper.h @@ -851,7 +851,6 @@ int drm_dp_add_payload_part1(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_topology_state *mst_state, struct drm_dp_mst_atomic_payload *payload); int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr, - struct drm_atomic_state *state, struct drm_dp_mst_atomic_payload *payload); void drm_dp_remove_payload_part1(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_topology_state *mst_state,
[Public]
Ping for code review. Thanks!
Regards, Wayne
________________________________________ From: Wayne Lin Wayne.Lin@amd.com Sent: Thursday, March 7, 2024 14:29 To: dri-devel@lists.freedesktop.org; amd-gfx@lists.freedesktop.org; intel-gfx@lists.freedesktop.org Cc: lyude@redhat.com; Wentland, Harry; imre.deak@intel.com; Lin, Wayne; Leon Weiß; stable@vger.kernel.org; regressions@lists.linux.dev Subject: [PATCH] drm/mst: Fix NULL pointer dereference at drm_dp_add_payload_part2
[Why] Commit: - commit 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload allocation/removement") accidently overwrite the commit - commit 54d217406afe ("drm: use mgr->dev in drm_dbg_kms in drm_dp_add_payload_part2") which cause regression.
[How] Recover the original NULL fix and remove the unnecessary input parameter 'state' for drm_dp_add_payload_part2().
Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload allocation/removement") Reported-by: Leon Weiß leon.weiss@ruhr-uni-bochum.de Link: https://lore.kernel.org/r/38c253ea42072cc825dc969ac4e6b9b600371cc8.camel@ruh... Cc: lyude@redhat.com Cc: imre.deak@intel.com Cc: stable@vger.kernel.org Cc: regressions@lists.linux.dev Signed-off-by: Wayne Lin Wayne.Lin@amd.com --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +- drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- include/drm/display/drm_dp_mst_helper.h | 1 - 5 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index c27063305a13..2c36f3d00ca2 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -363,7 +363,7 @@ void dm_helpers_dp_mst_send_payload_allocation( mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state); new_payload = drm_atomic_get_mst_payload_state(mst_state, aconnector->mst_output_port);
- ret = drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, new_payload); + ret = drm_dp_add_payload_part2(mst_mgr, new_payload);
if (ret) { amdgpu_dm_set_mst_status(&aconnector->mst_status, diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c index 03d528209426..95fd18f24e94 100644 --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c @@ -3421,7 +3421,6 @@ EXPORT_SYMBOL(drm_dp_remove_payload_part2); /** * drm_dp_add_payload_part2() - Execute payload update part 2 * @mgr: Manager to use. - * @state: The global atomic state * @payload: The payload to update * * If @payload was successfully assigned a starting time slot by drm_dp_add_payload_part1(), this @@ -3430,14 +3429,13 @@ EXPORT_SYMBOL(drm_dp_remove_payload_part2); * Returns: 0 on success, negative error code on failure. */ int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr, - struct drm_atomic_state *state, struct drm_dp_mst_atomic_payload *payload) { int ret = 0;
/* Skip failed payloads */ if (payload->payload_allocation_status != DRM_DP_MST_PAYLOAD_ALLOCATION_DFP) { - drm_dbg_kms(state->dev, "Part 1 of payload creation for %s failed, skipping part 2\n", + drm_dbg_kms(mgr->dev, "Part 1 of payload creation for %s failed, skipping part 2\n", payload->port->connector->name); return -EIO; } diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 53aec023ce92..2fba66aec038 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -1160,7 +1160,7 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state, if (first_mst_stream) intel_ddi_wait_for_fec_status(encoder, pipe_config, true);
- drm_dp_add_payload_part2(&intel_dp->mst_mgr, &state->base, + drm_dp_add_payload_part2(&intel_dp->mst_mgr, drm_atomic_get_mst_payload_state(mst_state, connector->port));
if (DISPLAY_VER(dev_priv) >= 12) diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 0c3d88ad0b0e..88728a0b2c25 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -915,7 +915,7 @@ nv50_msto_cleanup(struct drm_atomic_state *state, msto->disabled = false; drm_dp_remove_payload_part2(mgr, new_mst_state, old_payload, new_payload); } else if (msto->enabled) { - drm_dp_add_payload_part2(mgr, state, new_payload); + drm_dp_add_payload_part2(mgr, new_payload); msto->enabled = false; } } diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h index 9b19d8bd520a..6c9145abc7e2 100644 --- a/include/drm/display/drm_dp_mst_helper.h +++ b/include/drm/display/drm_dp_mst_helper.h @@ -851,7 +851,6 @@ int drm_dp_add_payload_part1(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_topology_state *mst_state, struct drm_dp_mst_atomic_payload *payload); int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr, - struct drm_atomic_state *state, struct drm_dp_mst_atomic_payload *payload); void drm_dp_remove_payload_part1(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_topology_state *mst_state, -- 2.37.3
On 2024-03-07 01:29, Wayne Lin wrote:
[Why] Commit:
- commit 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload allocation/removement")
accidently overwrite the commit
- commit 54d217406afe ("drm: use mgr->dev in drm_dbg_kms in drm_dp_add_payload_part2")
which cause regression.
[How] Recover the original NULL fix and remove the unnecessary input parameter 'state' for drm_dp_add_payload_part2().
Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload allocation/removement") Reported-by: Leon Weiß leon.weiss@ruhr-uni-bochum.de Link: https://lore.kernel.org/r/38c253ea42072cc825dc969ac4e6b9b600371cc8.camel@ruh... Cc: lyude@redhat.com Cc: imre.deak@intel.com Cc: stable@vger.kernel.org Cc: regressions@lists.linux.dev Signed-off-by: Wayne Lin Wayne.Lin@amd.com
I haven't been deep in MST code in a while but this all looks pretty straightforward and good.
Reviewed-by: Harry Wentland harry.wentland@amd.com
Harry
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +- drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- include/drm/display/drm_dp_mst_helper.h | 1 - 5 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index c27063305a13..2c36f3d00ca2 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -363,7 +363,7 @@ void dm_helpers_dp_mst_send_payload_allocation( mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state); new_payload = drm_atomic_get_mst_payload_state(mst_state, aconnector->mst_output_port);
- ret = drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, new_payload);
- ret = drm_dp_add_payload_part2(mst_mgr, new_payload);
if (ret) { amdgpu_dm_set_mst_status(&aconnector->mst_status, diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c index 03d528209426..95fd18f24e94 100644 --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c @@ -3421,7 +3421,6 @@ EXPORT_SYMBOL(drm_dp_remove_payload_part2); /**
- drm_dp_add_payload_part2() - Execute payload update part 2
- @mgr: Manager to use.
- @state: The global atomic state
- @payload: The payload to update
- If @payload was successfully assigned a starting time slot by drm_dp_add_payload_part1(), this
@@ -3430,14 +3429,13 @@ EXPORT_SYMBOL(drm_dp_remove_payload_part2);
- Returns: 0 on success, negative error code on failure.
*/ int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr,
struct drm_atomic_state *state, struct drm_dp_mst_atomic_payload *payload){ int ret = 0; /* Skip failed payloads */ if (payload->payload_allocation_status != DRM_DP_MST_PAYLOAD_ALLOCATION_DFP) {
drm_dbg_kms(state->dev, "Part 1 of payload creation for %s failed, skipping part 2\n",
drm_dbg_kms(mgr->dev, "Part 1 of payload creation for %s failed, skipping part 2\n", payload->port->connector->name);diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 53aec023ce92..2fba66aec038 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -1160,7 +1160,7 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state, if (first_mst_stream) intel_ddi_wait_for_fec_status(encoder, pipe_config, true);
- drm_dp_add_payload_part2(&intel_dp->mst_mgr, &state->base,
- drm_dp_add_payload_part2(&intel_dp->mst_mgr, drm_atomic_get_mst_payload_state(mst_state, connector->port));
if (DISPLAY_VER(dev_priv) >= 12) diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 0c3d88ad0b0e..88728a0b2c25 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -915,7 +915,7 @@ nv50_msto_cleanup(struct drm_atomic_state *state, msto->disabled = false; drm_dp_remove_payload_part2(mgr, new_mst_state, old_payload, new_payload); } else if (msto->enabled) {
drm_dp_add_payload_part2(mgr, state, new_payload);
drm_dp_add_payload_part2(mgr, new_payload);} diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h index 9b19d8bd520a..6c9145abc7e2 100644 --- a/include/drm/display/drm_dp_mst_helper.h +++ b/include/drm/display/drm_dp_mst_helper.h @@ -851,7 +851,6 @@ int drm_dp_add_payload_part1(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_topology_state *mst_state, struct drm_dp_mst_atomic_payload *payload); int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr,
struct drm_atomic_state *state, struct drm_dp_mst_atomic_payload *payload);void drm_dp_remove_payload_part1(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_topology_state *mst_state,
On 18.04.24 21:43, Harry Wentland wrote:
On 2024-03-07 01:29, Wayne Lin wrote:
[Why] Commit:
- commit 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload allocation/removement")
accidently overwrite the commit
- commit 54d217406afe ("drm: use mgr->dev in drm_dbg_kms in drm_dp_add_payload_part2")
which cause regression.
[How] Recover the original NULL fix and remove the unnecessary input parameter 'state' for drm_dp_add_payload_part2().
Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload allocation/removement") Reported-by: Leon Weiß leon.weiss@ruhr-uni-bochum.de Link: https://lore.kernel.org/r/38c253ea42072cc825dc969ac4e6b9b600371cc8.camel@ruh... Cc: lyude@redhat.com Cc: imre.deak@intel.com Cc: stable@vger.kernel.org Cc: regressions@lists.linux.dev Signed-off-by: Wayne Lin Wayne.Lin@amd.com
I haven't been deep in MST code in a while but this all looks pretty straightforward and good.
Reviewed-by: Harry Wentland harry.wentland@amd.com
Hmmm, that was three weeks ago, but it seems since then nothing happened to fix the linked regression through this or some other patch. Is there a reason? The build failure report from the CI maybe?
Wayne Lin, do you know what's up?
Ciao, Thorsten
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +- drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- include/drm/display/drm_dp_mst_helper.h | 1 - 5 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index c27063305a13..2c36f3d00ca2 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -363,7 +363,7 @@ void dm_helpers_dp_mst_send_payload_allocation( mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state); new_payload = drm_atomic_get_mst_payload_state(mst_state, aconnector->mst_output_port);
- ret = drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, new_payload);
- ret = drm_dp_add_payload_part2(mst_mgr, new_payload);
if (ret) { amdgpu_dm_set_mst_status(&aconnector->mst_status, diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c index 03d528209426..95fd18f24e94 100644 --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c @@ -3421,7 +3421,6 @@ EXPORT_SYMBOL(drm_dp_remove_payload_part2); /**
- drm_dp_add_payload_part2() - Execute payload update part 2
- @mgr: Manager to use.
- @state: The global atomic state
- @payload: The payload to update
- If @payload was successfully assigned a starting time slot by drm_dp_add_payload_part1(), this
@@ -3430,14 +3429,13 @@ EXPORT_SYMBOL(drm_dp_remove_payload_part2);
- Returns: 0 on success, negative error code on failure.
*/ int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr,
struct drm_atomic_state *state, struct drm_dp_mst_atomic_payload *payload){ int ret = 0; /* Skip failed payloads */ if (payload->payload_allocation_status != DRM_DP_MST_PAYLOAD_ALLOCATION_DFP) {
drm_dbg_kms(state->dev, "Part 1 of payload creation for %s failed, skipping part 2\n",
drm_dbg_kms(mgr->dev, "Part 1 of payload creation for %s failed, skipping part 2\n", payload->port->connector->name);diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 53aec023ce92..2fba66aec038 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -1160,7 +1160,7 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state, if (first_mst_stream) intel_ddi_wait_for_fec_status(encoder, pipe_config, true);
- drm_dp_add_payload_part2(&intel_dp->mst_mgr, &state->base,
- drm_dp_add_payload_part2(&intel_dp->mst_mgr, drm_atomic_get_mst_payload_state(mst_state, connector->port));
if (DISPLAY_VER(dev_priv) >= 12) diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 0c3d88ad0b0e..88728a0b2c25 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -915,7 +915,7 @@ nv50_msto_cleanup(struct drm_atomic_state *state, msto->disabled = false; drm_dp_remove_payload_part2(mgr, new_mst_state, old_payload, new_payload); } else if (msto->enabled) {
drm_dp_add_payload_part2(mgr, state, new_payload);
drm_dp_add_payload_part2(mgr, new_payload);} diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h index 9b19d8bd520a..6c9145abc7e2 100644 --- a/include/drm/display/drm_dp_mst_helper.h +++ b/include/drm/display/drm_dp_mst_helper.h @@ -851,7 +851,6 @@ int drm_dp_add_payload_part1(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_topology_state *mst_state, struct drm_dp_mst_atomic_payload *payload); int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr,
struct drm_atomic_state *state, struct drm_dp_mst_atomic_payload *payload);void drm_dp_remove_payload_part1(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_topology_state *mst_state,
On 5/9/2024 07:43, Linux regression tracking (Thorsten Leemhuis) wrote:
On 18.04.24 21:43, Harry Wentland wrote:
On 2024-03-07 01:29, Wayne Lin wrote:
[Why] Commit:
- commit 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload allocation/removement")
accidently overwrite the commit
- commit 54d217406afe ("drm: use mgr->dev in drm_dbg_kms in drm_dp_add_payload_part2")
which cause regression.
[How] Recover the original NULL fix and remove the unnecessary input parameter 'state' for drm_dp_add_payload_part2().
Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload allocation/removement") Reported-by: Leon Weiß leon.weiss@ruhr-uni-bochum.de Link: https://lore.kernel.org/r/38c253ea42072cc825dc969ac4e6b9b600371cc8.camel@ruh... Cc: lyude@redhat.com Cc: imre.deak@intel.com Cc: stable@vger.kernel.org Cc: regressions@lists.linux.dev Signed-off-by: Wayne Lin Wayne.Lin@amd.com
I haven't been deep in MST code in a while but this all looks pretty straightforward and good.
Reviewed-by: Harry Wentland harry.wentland@amd.com
Hmmm, that was three weeks ago, but it seems since then nothing happened to fix the linked regression through this or some other patch. Is there a reason? The build failure report from the CI maybe?
It touches files outside of amd but only has an ack from AMD. I think we /probably/ want an ack from i915 and nouveau to take it through.
Wayne Lin, do you know what's up?
Ciao, Thorsten
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +- drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- include/drm/display/drm_dp_mst_helper.h | 1 - 5 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index c27063305a13..2c36f3d00ca2 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -363,7 +363,7 @@ void dm_helpers_dp_mst_send_payload_allocation( mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state); new_payload = drm_atomic_get_mst_payload_state(mst_state, aconnector->mst_output_port);
- ret = drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, new_payload);
- ret = drm_dp_add_payload_part2(mst_mgr, new_payload);
if (ret) { amdgpu_dm_set_mst_status(&aconnector->mst_status, diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c index 03d528209426..95fd18f24e94 100644 --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c @@ -3421,7 +3421,6 @@ EXPORT_SYMBOL(drm_dp_remove_payload_part2); /**
- drm_dp_add_payload_part2() - Execute payload update part 2
- @mgr: Manager to use.
- @state: The global atomic state
- @payload: The payload to update
- If @payload was successfully assigned a starting time slot by drm_dp_add_payload_part1(), this
@@ -3430,14 +3429,13 @@ EXPORT_SYMBOL(drm_dp_remove_payload_part2);
- Returns: 0 on success, negative error code on failure.
*/ int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr,
struct drm_atomic_state *state, struct drm_dp_mst_atomic_payload *payload)/* Skip failed payloads */ if (payload->payload_allocation_status != DRM_DP_MST_PAYLOAD_ALLOCATION_DFP) {
drm_dbg_kms(state->dev, "Part 1 of payload creation for %s failed, skipping part 2\n",
drm_dbg_kms(mgr->dev, "Part 1 of payload creation for %s failed, skipping part 2\n", payload->port->connector->name);diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 53aec023ce92..2fba66aec038 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -1160,7 +1160,7 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state, if (first_mst_stream) intel_ddi_wait_for_fec_status(encoder, pipe_config, true);
- drm_dp_add_payload_part2(&intel_dp->mst_mgr, &state->base,
- drm_dp_add_payload_part2(&intel_dp->mst_mgr, drm_atomic_get_mst_payload_state(mst_state, connector->port));
if (DISPLAY_VER(dev_priv) >= 12) diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 0c3d88ad0b0e..88728a0b2c25 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -915,7 +915,7 @@ nv50_msto_cleanup(struct drm_atomic_state *state, msto->disabled = false; drm_dp_remove_payload_part2(mgr, new_mst_state, old_payload, new_payload); } else if (msto->enabled) {
drm_dp_add_payload_part2(mgr, state, new_payload);
drm_dp_add_payload_part2(mgr, new_payload);diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h index 9b19d8bd520a..6c9145abc7e2 100644 --- a/include/drm/display/drm_dp_mst_helper.h +++ b/include/drm/display/drm_dp_mst_helper.h @@ -851,7 +851,6 @@ int drm_dp_add_payload_part1(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_topology_state *mst_state, struct drm_dp_mst_atomic_payload *payload); int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr,
struct drm_atomic_state *state, struct drm_dp_mst_atomic_payload *payload);
[Public]
-----Original Message----- From: Limonciello, Mario Mario.Limonciello@amd.com Sent: Friday, May 10, 2024 3:18 AM To: Linux regressions mailing list regressions@lists.linux.dev; Wentland, Harry Harry.Wentland@amd.com; Lin, Wayne Wayne.Lin@amd.com Cc: lyude@redhat.com; imre.deak@intel.com; Leon Weiß <leon.weiss@ruhr-uni- bochum.de>; stable@vger.kernel.org; dri-devel@lists.freedesktop.org; amd- gfx@lists.freedesktop.org; intel-gfx@lists.freedesktop.org Subject: Re: [PATCH] drm/mst: Fix NULL pointer dereference at drm_dp_add_payload_part2
On 5/9/2024 07:43, Linux regression tracking (Thorsten Leemhuis) wrote:
On 18.04.24 21:43, Harry Wentland wrote:
On 2024-03-07 01:29, Wayne Lin wrote:
[Why] Commit:
- commit 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload
allocation/removement") accidently overwrite the commit
- commit 54d217406afe ("drm: use mgr->dev in drm_dbg_kms in
drm_dp_add_payload_part2") which cause regression.
[How] Recover the original NULL fix and remove the unnecessary input parameter 'state' for drm_dp_add_payload_part2().
Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload allocation/removement") Reported-by: Leon Weiß leon.weiss@ruhr-uni-bochum.de Link: https://lore.kernel.org/r/38c253ea42072cc825dc969ac4e6b9b600371cc8.c amel@ruhr-uni-bochum.de/ Cc: lyude@redhat.com Cc: imre.deak@intel.com Cc: stable@vger.kernel.org Cc: regressions@lists.linux.dev Signed-off-by: Wayne Lin Wayne.Lin@amd.com
I haven't been deep in MST code in a while but this all looks pretty straightforward and good.
Reviewed-by: Harry Wentland harry.wentland@amd.com
Hmmm, that was three weeks ago, but it seems since then nothing happened to fix the linked regression through this or some other patch. Is there a reason? The build failure report from the CI maybe?
It touches files outside of amd but only has an ack from AMD. I think we /probably/ want an ack from i915 and nouveau to take it through.
Thanks, Mario!
Hi Thorsten, Yeah, like what Mario said. Would also like to have ack from i915 and nouveau.
Wayne Lin, do you know what's up?
Ciao, Thorsten
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +- drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- include/drm/display/drm_dp_mst_helper.h | 1 - 5 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index c27063305a13..2c36f3d00ca2 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -363,7 +363,7 @@ void dm_helpers_dp_mst_send_payload_allocation( mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state); new_payload = drm_atomic_get_mst_payload_state(mst_state, aconnector->mst_output_port);
- ret = drm_dp_add_payload_part2(mst_mgr, mst_state->base.state,
new_payload);
ret = drm_dp_add_payload_part2(mst_mgr, new_payload);
if (ret) { amdgpu_dm_set_mst_status(&aconnector->mst_status,diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c index 03d528209426..95fd18f24e94 100644 --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c @@ -3421,7 +3421,6 @@
EXPORT_SYMBOL(drm_dp_remove_payload_part2);
/**
- drm_dp_add_payload_part2() - Execute payload update part 2
- @mgr: Manager to use.
- @state: The global atomic state
- @payload: The payload to update
- If @payload was successfully assigned a starting time slot by
drm_dp_add_payload_part1(), this @@ -3430,14 +3429,13 @@
EXPORT_SYMBOL(drm_dp_remove_payload_part2);
- Returns: 0 on success, negative error code on failure.
*/ int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr,
struct drm_atomic_state *state, struct drm_dp_mst_atomic_payload *payload){ int ret = 0;
/* Skip failed payloads */ if (payload->payload_allocation_status !=DRM_DP_MST_PAYLOAD_ALLOCATION_DFP) {
drm_dbg_kms(state->dev, "Part 1 of payload creation for %sfailed, skipping part 2\n",
drm_dbg_kms(mgr->dev, "Part 1 of payload creation for %sfailed,
+skipping part 2\n", payload->port->connector->name); return -EIO; } diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 53aec023ce92..2fba66aec038 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -1160,7 +1160,7 @@ static void intel_mst_enable_dp(struct
intel_atomic_state *state,
if (first_mst_stream) intel_ddi_wait_for_fec_status(encoder, pipe_config, true);
- drm_dp_add_payload_part2(&intel_dp->mst_mgr, &state->base,
- drm_dp_add_payload_part2(&intel_dp->mst_mgr,
drm_atomic_get_mst_payload_state(mst_state,
connector->port));
if (DISPLAY_VER(dev_priv) >= 12)diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 0c3d88ad0b0e..88728a0b2c25 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -915,7 +915,7 @@ nv50_msto_cleanup(struct drm_atomic_state *state, msto->disabled = false; drm_dp_remove_payload_part2(mgr, new_mst_state,
old_payload, new_payload);
} else if (msto->enabled) {
drm_dp_add_payload_part2(mgr, state, new_payload);
drm_dp_add_payload_part2(mgr, new_payload); msto->enabled = false; }diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h index 9b19d8bd520a..6c9145abc7e2 100644 --- a/include/drm/display/drm_dp_mst_helper.h +++ b/include/drm/display/drm_dp_mst_helper.h @@ -851,7 +851,6 @@ int drm_dp_add_payload_part1(struct
drm_dp_mst_topology_mgr *mgr,
struct drm_dp_mst_topology_state *mst_state, struct drm_dp_mst_atomic_payload *payload);int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr,
struct drm_atomic_state *state, struct drm_dp_mst_atomic_payload *payload);*mgr,
struct drm_dp_mst_topology_state *mst_state,
--- Regards, Wayne Lin
On Fri, 10 May 2024, "Lin, Wayne" Wayne.Lin@amd.com wrote:
[Public]
-----Original Message----- From: Limonciello, Mario Mario.Limonciello@amd.com Sent: Friday, May 10, 2024 3:18 AM To: Linux regressions mailing list regressions@lists.linux.dev; Wentland, Harry Harry.Wentland@amd.com; Lin, Wayne Wayne.Lin@amd.com Cc: lyude@redhat.com; imre.deak@intel.com; Leon Weiß <leon.weiss@ruhr-uni- bochum.de>; stable@vger.kernel.org; dri-devel@lists.freedesktop.org; amd- gfx@lists.freedesktop.org; intel-gfx@lists.freedesktop.org Subject: Re: [PATCH] drm/mst: Fix NULL pointer dereference at drm_dp_add_payload_part2
On 5/9/2024 07:43, Linux regression tracking (Thorsten Leemhuis) wrote:
On 18.04.24 21:43, Harry Wentland wrote:
On 2024-03-07 01:29, Wayne Lin wrote:
[Why] Commit:
- commit 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload
allocation/removement") accidently overwrite the commit
- commit 54d217406afe ("drm: use mgr->dev in drm_dbg_kms in
drm_dp_add_payload_part2") which cause regression.
[How] Recover the original NULL fix and remove the unnecessary input parameter 'state' for drm_dp_add_payload_part2().
Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload allocation/removement") Reported-by: Leon Weiß leon.weiss@ruhr-uni-bochum.de Link: https://lore.kernel.org/r/38c253ea42072cc825dc969ac4e6b9b600371cc8.c amel@ruhr-uni-bochum.de/ Cc: lyude@redhat.com Cc: imre.deak@intel.com Cc: stable@vger.kernel.org Cc: regressions@lists.linux.dev Signed-off-by: Wayne Lin Wayne.Lin@amd.com
I haven't been deep in MST code in a while but this all looks pretty straightforward and good.
Reviewed-by: Harry Wentland harry.wentland@amd.com
Hmmm, that was three weeks ago, but it seems since then nothing happened to fix the linked regression through this or some other patch. Is there a reason? The build failure report from the CI maybe?
It touches files outside of amd but only has an ack from AMD. I think we /probably/ want an ack from i915 and nouveau to take it through.
Thanks, Mario!
Hi Thorsten, Yeah, like what Mario said. Would also like to have ack from i915 and nouveau.
It usually works better if you Cc the folks you want an ack from! ;)
Acked-by: Jani Nikula jani.nikula@intel.com
Wayne Lin, do you know what's up?
Ciao, Thorsten
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +- drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- include/drm/display/drm_dp_mst_helper.h | 1 - 5 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index c27063305a13..2c36f3d00ca2 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -363,7 +363,7 @@ void dm_helpers_dp_mst_send_payload_allocation( mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state); new_payload = drm_atomic_get_mst_payload_state(mst_state, aconnector->mst_output_port);
- ret = drm_dp_add_payload_part2(mst_mgr, mst_state->base.state,
new_payload);
ret = drm_dp_add_payload_part2(mst_mgr, new_payload);
if (ret) { amdgpu_dm_set_mst_status(&aconnector->mst_status,diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c index 03d528209426..95fd18f24e94 100644 --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c @@ -3421,7 +3421,6 @@
EXPORT_SYMBOL(drm_dp_remove_payload_part2);
/**
- drm_dp_add_payload_part2() - Execute payload update part 2
- @mgr: Manager to use.
- @state: The global atomic state
- @payload: The payload to update
- If @payload was successfully assigned a starting time slot by
drm_dp_add_payload_part1(), this @@ -3430,14 +3429,13 @@
EXPORT_SYMBOL(drm_dp_remove_payload_part2);
- Returns: 0 on success, negative error code on failure.
*/ int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr,
struct drm_atomic_state *state, struct drm_dp_mst_atomic_payload *payload){ int ret = 0;
/* Skip failed payloads */ if (payload->payload_allocation_status !=DRM_DP_MST_PAYLOAD_ALLOCATION_DFP) {
drm_dbg_kms(state->dev, "Part 1 of payload creation for %sfailed, skipping part 2\n",
drm_dbg_kms(mgr->dev, "Part 1 of payload creation for %sfailed,
+skipping part 2\n", payload->port->connector->name); return -EIO; } diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 53aec023ce92..2fba66aec038 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -1160,7 +1160,7 @@ static void intel_mst_enable_dp(struct
intel_atomic_state *state,
if (first_mst_stream) intel_ddi_wait_for_fec_status(encoder, pipe_config, true);
- drm_dp_add_payload_part2(&intel_dp->mst_mgr, &state->base,
- drm_dp_add_payload_part2(&intel_dp->mst_mgr,
drm_atomic_get_mst_payload_state(mst_state,
connector->port));
if (DISPLAY_VER(dev_priv) >= 12)diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 0c3d88ad0b0e..88728a0b2c25 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -915,7 +915,7 @@ nv50_msto_cleanup(struct drm_atomic_state *state, msto->disabled = false; drm_dp_remove_payload_part2(mgr, new_mst_state,
old_payload, new_payload);
} else if (msto->enabled) {
drm_dp_add_payload_part2(mgr, state, new_payload);
drm_dp_add_payload_part2(mgr, new_payload); msto->enabled = false; }diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h index 9b19d8bd520a..6c9145abc7e2 100644 --- a/include/drm/display/drm_dp_mst_helper.h +++ b/include/drm/display/drm_dp_mst_helper.h @@ -851,7 +851,6 @@ int drm_dp_add_payload_part1(struct
drm_dp_mst_topology_mgr *mgr,
struct drm_dp_mst_topology_state *mst_state, struct drm_dp_mst_atomic_payload *payload);int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr,
struct drm_atomic_state *state, struct drm_dp_mst_atomic_payload *payload);*mgr,
struct drm_dp_mst_topology_state *mst_state,
Regards, Wayne Lin
On 5/10/2024 4:24 AM, Jani Nikula wrote:
On Fri, 10 May 2024, "Lin, Wayne" Wayne.Lin@amd.com wrote:
[Public]
-----Original Message----- From: Limonciello, Mario Mario.Limonciello@amd.com Sent: Friday, May 10, 2024 3:18 AM To: Linux regressions mailing list regressions@lists.linux.dev; Wentland, Harry Harry.Wentland@amd.com; Lin, Wayne Wayne.Lin@amd.com Cc: lyude@redhat.com; imre.deak@intel.com; Leon Weiß <leon.weiss@ruhr-uni- bochum.de>; stable@vger.kernel.org; dri-devel@lists.freedesktop.org; amd- gfx@lists.freedesktop.org; intel-gfx@lists.freedesktop.org Subject: Re: [PATCH] drm/mst: Fix NULL pointer dereference at drm_dp_add_payload_part2
On 5/9/2024 07:43, Linux regression tracking (Thorsten Leemhuis) wrote:
On 18.04.24 21:43, Harry Wentland wrote:
On 2024-03-07 01:29, Wayne Lin wrote:
[Why] Commit:
- commit 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload
allocation/removement") accidently overwrite the commit
- commit 54d217406afe ("drm: use mgr->dev in drm_dbg_kms in
drm_dp_add_payload_part2") which cause regression.
[How] Recover the original NULL fix and remove the unnecessary input parameter 'state' for drm_dp_add_payload_part2().
Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload allocation/removement") Reported-by: Leon Weiß leon.weiss@ruhr-uni-bochum.de Link: https://lore.kernel.org/r/38c253ea42072cc825dc969ac4e6b9b600371cc8.c amel@ruhr-uni-bochum.de/ Cc: lyude@redhat.com Cc: imre.deak@intel.com Cc: stable@vger.kernel.org Cc: regressions@lists.linux.dev Signed-off-by: Wayne Lin Wayne.Lin@amd.com
I haven't been deep in MST code in a while but this all looks pretty straightforward and good.
Reviewed-by: Harry Wentland harry.wentland@amd.com
Hmmm, that was three weeks ago, but it seems since then nothing happened to fix the linked regression through this or some other patch. Is there a reason? The build failure report from the CI maybe?
It touches files outside of amd but only has an ack from AMD. I think we /probably/ want an ack from i915 and nouveau to take it through.
Thanks, Mario!
Hi Thorsten, Yeah, like what Mario said. Would also like to have ack from i915 and nouveau.
It usually works better if you Cc the folks you want an ack from! ;)
Acked-by: Jani Nikula jani.nikula@intel.com
Thanks! Can someone with commit permissions take this to drm-misc?
Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting for once, to make this easily accessible to everyone.
Hmm, from here it looks like the patch now that it was reviewed more that a week ago is still not even in -next. Is there a reason?
I know, we are in the merge window. But at the same time this is a fix (that already lingered on the lists for way too long before it was reviewed) for a regression in a somewhat recent kernel, so it in Linus own words should be "expedited"[1].
Or are we again just missing a right person for the job in the CC? Adding Dave and Sima just in case.
Ciao, Thorsten
[1] https://lore.kernel.org/all/CAHk-=wis_qQy4oDNynNKi5b7Qhosmxtoj1jxo5wmB6SRUwQ...
On 12.05.24 18:11, Limonciello, Mario wrote:
On 5/10/2024 4:24 AM, Jani Nikula wrote:
On Fri, 10 May 2024, "Lin, Wayne" Wayne.Lin@amd.com wrote:
-----Original Message----- From: Limonciello, Mario Mario.Limonciello@amd.com Sent: Friday, May 10, 2024 3:18 AM To: Linux regressions mailing list regressions@lists.linux.dev; Wentland, Harry Harry.Wentland@amd.com; Lin, Wayne Wayne.Lin@amd.com Cc: lyude@redhat.com; imre.deak@intel.com; Leon Weiß <leon.weiss@ruhr-uni- bochum.de>; stable@vger.kernel.org; dri-devel@lists.freedesktop.org; amd- gfx@lists.freedesktop.org; intel-gfx@lists.freedesktop.org Subject: Re: [PATCH] drm/mst: Fix NULL pointer dereference at drm_dp_add_payload_part2
On 5/9/2024 07:43, Linux regression tracking (Thorsten Leemhuis) wrote:
On 18.04.24 21:43, Harry Wentland wrote:
On 2024-03-07 01:29, Wayne Lin wrote: > [Why] > Commit: > - commit 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload > allocation/removement") accidently overwrite the commit > - commit 54d217406afe ("drm: use mgr->dev in drm_dbg_kms in > drm_dp_add_payload_part2") which cause regression. > > [How] > Recover the original NULL fix and remove the unnecessary input > parameter 'state' for drm_dp_add_payload_part2(). > > Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload > allocation/removement") > Reported-by: Leon Weiß leon.weiss@ruhr-uni-bochum.de > Link: > https://lore.kernel.org/r/38c253ea42072cc825dc969ac4e6b9b600371cc8.c > amel@ruhr-uni-bochum.de/ > Cc: lyude@redhat.com > Cc: imre.deak@intel.com > Cc: stable@vger.kernel.org > Cc: regressions@lists.linux.dev > Signed-off-by: Wayne Lin Wayne.Lin@amd.com
I haven't been deep in MST code in a while but this all looks pretty straightforward and good.
Reviewed-by: Harry Wentland harry.wentland@amd.com
Hmmm, that was three weeks ago, but it seems since then nothing happened to fix the linked regression through this or some other patch. Is there a reason? The build failure report from the CI maybe?
It touches files outside of amd but only has an ack from AMD. I think we /probably/ want an ack from i915 and nouveau to take it through.
Thanks, Mario!
Hi Thorsten, Yeah, like what Mario said. Would also like to have ack from i915 and nouveau.
It usually works better if you Cc the folks you want an ack from! ;)
Acked-by: Jani Nikula jani.nikula@intel.com
Thanks! Can someone with commit permissions take this to drm-misc?
I've got it teed up. Is drm-misc-fixes the right branch since we are in the merge window?
Alex
On Tue, May 21, 2024 at 7:20 AM Linux regression tracking (Thorsten Leemhuis) regressions@leemhuis.info wrote:
Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting for once, to make this easily accessible to everyone.
Hmm, from here it looks like the patch now that it was reviewed more that a week ago is still not even in -next. Is there a reason?
I know, we are in the merge window. But at the same time this is a fix (that already lingered on the lists for way too long before it was reviewed) for a regression in a somewhat recent kernel, so it in Linus own words should be "expedited"[1].
Or are we again just missing a right person for the job in the CC? Adding Dave and Sima just in case.
Ciao, Thorsten
[1] https://lore.kernel.org/all/CAHk-=wis_qQy4oDNynNKi5b7Qhosmxtoj1jxo5wmB6SRUwQ...
On 12.05.24 18:11, Limonciello, Mario wrote:
On 5/10/2024 4:24 AM, Jani Nikula wrote:
On Fri, 10 May 2024, "Lin, Wayne" Wayne.Lin@amd.com wrote:
-----Original Message----- From: Limonciello, Mario Mario.Limonciello@amd.com Sent: Friday, May 10, 2024 3:18 AM To: Linux regressions mailing list regressions@lists.linux.dev; Wentland, Harry Harry.Wentland@amd.com; Lin, Wayne Wayne.Lin@amd.com Cc: lyude@redhat.com; imre.deak@intel.com; Leon Weiß <leon.weiss@ruhr-uni- bochum.de>; stable@vger.kernel.org; dri-devel@lists.freedesktop.org; amd- gfx@lists.freedesktop.org; intel-gfx@lists.freedesktop.org Subject: Re: [PATCH] drm/mst: Fix NULL pointer dereference at drm_dp_add_payload_part2
On 5/9/2024 07:43, Linux regression tracking (Thorsten Leemhuis) wrote:
On 18.04.24 21:43, Harry Wentland wrote: > On 2024-03-07 01:29, Wayne Lin wrote: >> [Why] >> Commit: >> - commit 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload >> allocation/removement") accidently overwrite the commit >> - commit 54d217406afe ("drm: use mgr->dev in drm_dbg_kms in >> drm_dp_add_payload_part2") which cause regression. >> >> [How] >> Recover the original NULL fix and remove the unnecessary input >> parameter 'state' for drm_dp_add_payload_part2(). >> >> Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload >> allocation/removement") >> Reported-by: Leon Weiß leon.weiss@ruhr-uni-bochum.de >> Link: >> https://lore.kernel.org/r/38c253ea42072cc825dc969ac4e6b9b600371cc8.c >> amel@ruhr-uni-bochum.de/ >> Cc: lyude@redhat.com >> Cc: imre.deak@intel.com >> Cc: stable@vger.kernel.org >> Cc: regressions@lists.linux.dev >> Signed-off-by: Wayne Lin Wayne.Lin@amd.com > > I haven't been deep in MST code in a while but this all looks pretty > straightforward and good. > > Reviewed-by: Harry Wentland harry.wentland@amd.com
Hmmm, that was three weeks ago, but it seems since then nothing happened to fix the linked regression through this or some other patch. Is there a reason? The build failure report from the CI maybe?
It touches files outside of amd but only has an ack from AMD. I think we /probably/ want an ack from i915 and nouveau to take it through.
Thanks, Mario!
Hi Thorsten, Yeah, like what Mario said. Would also like to have ack from i915 and nouveau.
It usually works better if you Cc the folks you want an ack from! ;)
Acked-by: Jani Nikula jani.nikula@intel.com
Thanks! Can someone with commit permissions take this to drm-misc?
Applied and pushed out: https://cgit.freedesktop.org/drm/drm-misc/commit/?id=8a0a7b98d4b6eeeab337ec2...
Alex
On Tue, May 21, 2024 at 12:12 PM Alex Deucher alexdeucher@gmail.com wrote:
I've got it teed up. Is drm-misc-fixes the right branch since we are in the merge window?
Alex
On Tue, May 21, 2024 at 7:20 AM Linux regression tracking (Thorsten Leemhuis) regressions@leemhuis.info wrote:
Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting for once, to make this easily accessible to everyone.
Hmm, from here it looks like the patch now that it was reviewed more that a week ago is still not even in -next. Is there a reason?
I know, we are in the merge window. But at the same time this is a fix (that already lingered on the lists for way too long before it was reviewed) for a regression in a somewhat recent kernel, so it in Linus own words should be "expedited"[1].
Or are we again just missing a right person for the job in the CC? Adding Dave and Sima just in case.
Ciao, Thorsten
[1] https://lore.kernel.org/all/CAHk-=wis_qQy4oDNynNKi5b7Qhosmxtoj1jxo5wmB6SRUwQ...
On 12.05.24 18:11, Limonciello, Mario wrote:
On 5/10/2024 4:24 AM, Jani Nikula wrote:
On Fri, 10 May 2024, "Lin, Wayne" Wayne.Lin@amd.com wrote:
-----Original Message----- From: Limonciello, Mario Mario.Limonciello@amd.com Sent: Friday, May 10, 2024 3:18 AM To: Linux regressions mailing list regressions@lists.linux.dev; Wentland, Harry Harry.Wentland@amd.com; Lin, Wayne Wayne.Lin@amd.com Cc: lyude@redhat.com; imre.deak@intel.com; Leon Weiß <leon.weiss@ruhr-uni- bochum.de>; stable@vger.kernel.org; dri-devel@lists.freedesktop.org; amd- gfx@lists.freedesktop.org; intel-gfx@lists.freedesktop.org Subject: Re: [PATCH] drm/mst: Fix NULL pointer dereference at drm_dp_add_payload_part2
On 5/9/2024 07:43, Linux regression tracking (Thorsten Leemhuis) wrote: > On 18.04.24 21:43, Harry Wentland wrote: >> On 2024-03-07 01:29, Wayne Lin wrote: >>> [Why] >>> Commit: >>> - commit 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload >>> allocation/removement") accidently overwrite the commit >>> - commit 54d217406afe ("drm: use mgr->dev in drm_dbg_kms in >>> drm_dp_add_payload_part2") which cause regression. >>> >>> [How] >>> Recover the original NULL fix and remove the unnecessary input >>> parameter 'state' for drm_dp_add_payload_part2(). >>> >>> Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload >>> allocation/removement") >>> Reported-by: Leon Weiß leon.weiss@ruhr-uni-bochum.de >>> Link: >>> https://lore.kernel.org/r/38c253ea42072cc825dc969ac4e6b9b600371cc8.c >>> amel@ruhr-uni-bochum.de/ >>> Cc: lyude@redhat.com >>> Cc: imre.deak@intel.com >>> Cc: stable@vger.kernel.org >>> Cc: regressions@lists.linux.dev >>> Signed-off-by: Wayne Lin Wayne.Lin@amd.com >> >> I haven't been deep in MST code in a while but this all looks pretty >> straightforward and good. >> >> Reviewed-by: Harry Wentland harry.wentland@amd.com > > Hmmm, that was three weeks ago, but it seems since then nothing > happened to fix the linked regression through this or some other > patch. Is there a reason? The build failure report from the CI maybe?
It touches files outside of amd but only has an ack from AMD. I think we /probably/ want an ack from i915 and nouveau to take it through.
Thanks, Mario!
Hi Thorsten, Yeah, like what Mario said. Would also like to have ack from i915 and nouveau.
It usually works better if you Cc the folks you want an ack from! ;)
Acked-by: Jani Nikula jani.nikula@intel.com
Thanks! Can someone with commit permissions take this to drm-misc?
linux-stable-mirror@lists.linaro.org
-
Alex Deucher -
Harry Wentland -
Jani Nikula -
Limonciello, Mario -
Lin, Wayne -
Linux regression tracking (Thorsten Leemhuis) -
Mario Limonciello
-
Wayne Lin