Re: [PATCH v3] drm/amd/display: turn DPMS off on mst connector unplug
[AMD Official Use Only - Internal Distribution Only]
Thank you very much for the review!
--
Thanks & Regards,
Aurabindo Pillai
From: Kazlauskas, Nicholas
Sent: Friday, November 27, 2020 10:44 AM
To: Pillai, Aurabindo ; amd-gfx@lists.freedesktop.org
Cc: Wentland, Harry ; Lakha, Bhawanpreet
; Brol, Eryk
Subject: Re: [PATCH v3] drm/amd/display: turn DPMS off on mst connector unplug
On 2020-11-26 7:18 p.m., Aurabindo Pillai wrote:
> [Why]
>
> Set dpms off on the MST connector that was unplugged, for the side effect of
> releasing some references held through deallocation of mst payload.
Applies to non-MST now too, so the description and title should be updated.
>
> Signed-off-by: Aurabindo Pillai
> Signed-off-by: Eryk Brol
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 31 ++-
> drivers/gpu/drm/amd/display/dc/core/dc.c | 13
> drivers/gpu/drm/amd/display/dc/dc_stream.h| 1 +
> 3 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index e213246e3f04..9966679d29e7 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1984,6 +1984,32 @@ static void dm_gpureset_commit_state(struct dc_state
> *dc_state,
>return;
> }
>
> +static void dm_set_dpms_off(struct dc_link *link)
> +{
> + struct dc_stream_state *stream_state;
> + struct amdgpu_dm_connector *aconnector = link->priv;
> + struct amdgpu_device *adev = drm_to_adev(aconnector->base.dev);
> + struct dc_stream_update stream_update = {0};
Please use a memset instead of a zero initializer here. Some compilers
complain about that.
> + bool dpms_off = true;
> +
> + stream_update.dpms_off = _off;
> +
> + mutex_lock(>dm.dc_lock);
> + stream_state = dc_stream_find_from_link(link);
> +
> + if (stream_state == NULL) {
> + dm_error("Error finding stream state associated with link!\n");
This shouldn't be using a dm_error print here. a DRM_DEBUG_DRIVER would
be better suited.
With these three items fixed the v4 of this patch will be:
Reviewed-by: Nicholas Kazlauskas
Regards,
Nicholas Kazlauskas
> + mutex_unlock(>dm.dc_lock);
> + return;
> + }
> +
> + stream_update.stream = stream_state;
> + dc_commit_updates_for_stream(stream_state->ctx->dc, NULL, 0,
> + stream_state, _update,
> + stream_state->ctx->dc->current_state);
> + mutex_unlock(>dm.dc_lock);
> +}
> +
> static int dm_resume(void *handle)
> {
>struct amdgpu_device *adev = handle;
> @@ -2434,8 +2460,11 @@ static void handle_hpd_irq(void *param)
>drm_kms_helper_hotplug_event(dev);
>
>} else if (dc_link_detect(aconnector->dc_link, DETECT_REASON_HPD)) {
> - amdgpu_dm_update_connector_after_detect(aconnector);
> + if (new_connection_type == dc_connection_none &&
> + aconnector->dc_link->type == dc_connection_none)
> + dm_set_dpms_off(aconnector->dc_link);
>
> + amdgpu_dm_update_connector_after_detect(aconnector);
>
>drm_modeset_lock_all(dev);
>dm_restore_drm_connector_state(dev, connector);
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
> b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index 903353389edb..58eb0d69873a 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -2798,6 +2798,19 @@ struct dc_stream_state *dc_get_stream_at_index(struct
> dc *dc, uint8_t i)
>return NULL;
> }
>
> +struct dc_stream_state *dc_stream_find_from_link(const struct dc_link *link)
> +{
> + uint8_t i;
> + struct dc_context *ctx = link->ctx;
> +
> + for (i = 0; i < ctx->dc->current_state->stream_count; i++) {
> + if (ctx->dc->current_state->streams[i]->link == link)
> + return ctx->dc->current_state->streams[i];
> + }
> +
> + return NULL;
> +}
> +
> enum dc_irq_source dc_interrupt_to_irq_source(
>struct dc *dc,
>uint32_t src_id,
> diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h
> b/drivers/gpu/drm/amd/display/dc/dc_stream.h
> index bf090afc2f70..b7910976b81a 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
> @@ -292,6 +292,7 @@ void dc_stre
Re: [PATCH v3] drm/amd/display: turn DPMS off on mst connector unplug
On 2020-11-26 7:18 p.m., Aurabindo Pillai wrote:
[Why]
Set dpms off on the MST connector that was unplugged, for the side effect of
releasing some references held through deallocation of mst payload.
Applies to non-MST now too, so the description and title should be updated.
Signed-off-by: Aurabindo Pillai
Signed-off-by: Eryk Brol
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 31 ++-
drivers/gpu/drm/amd/display/dc/core/dc.c | 13
drivers/gpu/drm/amd/display/dc/dc_stream.h| 1 +
3 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index e213246e3f04..9966679d29e7 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1984,6 +1984,32 @@ static void dm_gpureset_commit_state(struct dc_state
*dc_state,
return;
}
+static void dm_set_dpms_off(struct dc_link *link)
+{
+ struct dc_stream_state *stream_state;
+ struct amdgpu_dm_connector *aconnector = link->priv;
+ struct amdgpu_device *adev = drm_to_adev(aconnector->base.dev);
+ struct dc_stream_update stream_update = {0};
Please use a memset instead of a zero initializer here. Some compilers
complain about that.
+ bool dpms_off = true;
+
+ stream_update.dpms_off = _off;
+
+ mutex_lock(>dm.dc_lock);
+ stream_state = dc_stream_find_from_link(link);
+
+ if (stream_state == NULL) {
+ dm_error("Error finding stream state associated with link!\n");
This shouldn't be using a dm_error print here. a DRM_DEBUG_DRIVER would
be better suited.
With these three items fixed the v4 of this patch will be:
Reviewed-by: Nicholas Kazlauskas
Regards,
Nicholas Kazlauskas
+ mutex_unlock(>dm.dc_lock);
+ return;
+ }
+
+ stream_update.stream = stream_state;
+ dc_commit_updates_for_stream(stream_state->ctx->dc, NULL, 0,
+stream_state, _update,
+stream_state->ctx->dc->current_state);
+ mutex_unlock(>dm.dc_lock);
+}
+
static int dm_resume(void *handle)
{
struct amdgpu_device *adev = handle;
@@ -2434,8 +2460,11 @@ static void handle_hpd_irq(void *param)
drm_kms_helper_hotplug_event(dev);
} else if (dc_link_detect(aconnector->dc_link, DETECT_REASON_HPD)) {
- amdgpu_dm_update_connector_after_detect(aconnector);
+ if (new_connection_type == dc_connection_none &&
+ aconnector->dc_link->type == dc_connection_none)
+ dm_set_dpms_off(aconnector->dc_link);
+ amdgpu_dm_update_connector_after_detect(aconnector);
drm_modeset_lock_all(dev);
dm_restore_drm_connector_state(dev, connector);
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 903353389edb..58eb0d69873a 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -2798,6 +2798,19 @@ struct dc_stream_state *dc_get_stream_at_index(struct dc
*dc, uint8_t i)
return NULL;
}
+struct dc_stream_state *dc_stream_find_from_link(const struct dc_link *link)
+{
+ uint8_t i;
+ struct dc_context *ctx = link->ctx;
+
+ for (i = 0; i < ctx->dc->current_state->stream_count; i++) {
+ if (ctx->dc->current_state->streams[i]->link == link)
+ return ctx->dc->current_state->streams[i];
+ }
+
+ return NULL;
+}
+
enum dc_irq_source dc_interrupt_to_irq_source(
struct dc *dc,
uint32_t src_id,
diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h
b/drivers/gpu/drm/amd/display/dc/dc_stream.h
index bf090afc2f70..b7910976b81a 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
@@ -292,6 +292,7 @@ void dc_stream_log(const struct dc *dc, const struct
dc_stream_state *stream);
uint8_t dc_get_current_stream_count(struct dc *dc);
struct dc_stream_state *dc_get_stream_at_index(struct dc *dc, uint8_t i);
+struct dc_stream_state *dc_stream_find_from_link(const struct dc_link *link);
/*
* Return the current frame counter.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3] drm/amd/display: turn DPMS off on mst connector unplug
[Why]
Set dpms off on the MST connector that was unplugged, for the side effect of
releasing some references held through deallocation of mst payload.
Signed-off-by: Aurabindo Pillai
Signed-off-by: Eryk Brol
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 31 ++-
drivers/gpu/drm/amd/display/dc/core/dc.c | 13
drivers/gpu/drm/amd/display/dc/dc_stream.h| 1 +
3 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index e213246e3f04..9966679d29e7 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1984,6 +1984,32 @@ static void dm_gpureset_commit_state(struct dc_state
*dc_state,
return;
}
+static void dm_set_dpms_off(struct dc_link *link)
+{
+ struct dc_stream_state *stream_state;
+ struct amdgpu_dm_connector *aconnector = link->priv;
+ struct amdgpu_device *adev = drm_to_adev(aconnector->base.dev);
+ struct dc_stream_update stream_update = {0};
+ bool dpms_off = true;
+
+ stream_update.dpms_off = _off;
+
+ mutex_lock(>dm.dc_lock);
+ stream_state = dc_stream_find_from_link(link);
+
+ if (stream_state == NULL) {
+ dm_error("Error finding stream state associated with link!\n");
+ mutex_unlock(>dm.dc_lock);
+ return;
+ }
+
+ stream_update.stream = stream_state;
+ dc_commit_updates_for_stream(stream_state->ctx->dc, NULL, 0,
+stream_state, _update,
+stream_state->ctx->dc->current_state);
+ mutex_unlock(>dm.dc_lock);
+}
+
static int dm_resume(void *handle)
{
struct amdgpu_device *adev = handle;
@@ -2434,8 +2460,11 @@ static void handle_hpd_irq(void *param)
drm_kms_helper_hotplug_event(dev);
} else if (dc_link_detect(aconnector->dc_link, DETECT_REASON_HPD)) {
- amdgpu_dm_update_connector_after_detect(aconnector);
+ if (new_connection_type == dc_connection_none &&
+ aconnector->dc_link->type == dc_connection_none)
+ dm_set_dpms_off(aconnector->dc_link);
+ amdgpu_dm_update_connector_after_detect(aconnector);
drm_modeset_lock_all(dev);
dm_restore_drm_connector_state(dev, connector);
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 903353389edb..58eb0d69873a 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -2798,6 +2798,19 @@ struct dc_stream_state *dc_get_stream_at_index(struct dc
*dc, uint8_t i)
return NULL;
}
+struct dc_stream_state *dc_stream_find_from_link(const struct dc_link *link)
+{
+ uint8_t i;
+ struct dc_context *ctx = link->ctx;
+
+ for (i = 0; i < ctx->dc->current_state->stream_count; i++) {
+ if (ctx->dc->current_state->streams[i]->link == link)
+ return ctx->dc->current_state->streams[i];
+ }
+
+ return NULL;
+}
+
enum dc_irq_source dc_interrupt_to_irq_source(
struct dc *dc,
uint32_t src_id,
diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h
b/drivers/gpu/drm/amd/display/dc/dc_stream.h
index bf090afc2f70..b7910976b81a 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
@@ -292,6 +292,7 @@ void dc_stream_log(const struct dc *dc, const struct
dc_stream_state *stream);
uint8_t dc_get_current_stream_count(struct dc *dc);
struct dc_stream_state *dc_get_stream_at_index(struct dc *dc, uint8_t i);
+struct dc_stream_state *dc_stream_find_from_link(const struct dc_link *link);
/*
* Return the current frame counter.
--
2.25.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
