[PATCH v3] drm/amd/display: turn DPMS off on mst connector unplug

[Pillai, Aurabindo]

[AMD Official Use Only - Internal Distribution Only]

Thank you very much for the review!

--

Thanks & Regards,
Aurabindo Pillai
________________________________
From: Kazlauskas, Nicholas <Nicholas.Kazlauskas at amd.com>
Sent: Friday, November 27, 2020 10:44 AM
To: Pillai, Aurabindo <Aurabindo.Pillai at amd.com>; amd-gfx at lists.freedesktop.org <amd-gfx at lists.freedesktop.org>
Cc: Wentland, Harry <Harry.Wentland at amd.com>; Lakha, Bhawanpreet <Bhawanpreet.Lakha at amd.com>; Brol, Eryk <Eryk.Brol at amd.com>
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&How]
>
> 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 <aurabindo.pillai at amd.com>
> Signed-off-by: Eryk Brol <eryk.brol at amd.com>
> ---
>   .../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 = &dpms_off;
> +
> +     mutex_lock(&adev->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 <nicholas.kazlauskas at amd.com>

Regards,
Nicholas Kazlauskas

> +             mutex_unlock(&adev->dm.dc_lock);
> +             return;
> +     }
> +
> +     stream_update.stream = stream_state;
> +     dc_commit_updates_for_stream(stream_state->ctx->dc, NULL, 0,
> +                                  stream_state, &stream_update,
> +                                  stream_state->ctx->dc->current_state);
> +     mutex_unlock(&adev->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.
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20201127/77cf21d5/attachment.htm>