Re: [PATCH] drm/amd/display: Remove aconnector condition check for dpcd read

2020-04-20 Thread Gravenor, Joseph
[AMD Official Use Only - Internal Distribution Only]

Reviewed-by: Joseph Gravenor 

From: Liu, Zhan 
Sent: Friday, April 17, 2020 1:17 PM
To: Liu, Zhan ; amd-gfx@lists.freedesktop.org 
; Gravenor, Joseph 
Subject: RE: [PATCH] drm/amd/display: Remove aconnector condition check for 
dpcd read

+ Joseph



Hi Joseph,

Would you like to help me review this change? This was a follow-up on the 
discussion we had earlier this year.

Thanks,
Zhan


> -Original Message-
> From: Liu, Zhan 
> Sent: 2020/April/16, Thursday 3:24 PM
> To: amd-gfx@lists.freedesktop.org; Liu, Zhan 
> Subject: [PATCH] drm/amd/display: Remove aconnector condition check for
> dpcd read
>
> [Why]
> Aconnector is not necessary to be NULL in order to read dpcd successfully.
>
> Actually if we rely on checking aconnector here, we won't be able to turn off
> all displays before doing display detection. That will cause some MST hubs
> not able to light up.
>
> [How]
> Remove aconnector check when turning off all displays at hardware
> initialization stage.
>
> Signed-off-by: Zhan Liu 
> ---
>  .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 36 ---
>  1 file changed, 14 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> index 9f41efddc9bc..6f33f3f0d023 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> @@ -1332,31 +1332,23 @@ void dcn10_init_hw(struct dc *dc)
>if (dc->links[i]->connector_signal !=
> SIGNAL_TYPE_DISPLAY_PORT)
>continue;
>
> - /*
> -  * core_link_read_dpcd() will invoke
> dm_helpers_dp_read_dpcd(),
> -  * which needs to read dpcd info with the help of
> aconnector.
> -  * If aconnector (dc->links[i]->prev) is NULL, then
> dpcd status
> -  * cannot be read.
> -  */
> - if (dc->links[i]->priv) {
> - /* if any of the displays are lit up turn them
> off */
> - status = core_link_read_dpcd(dc->links[i],
> DP_SET_POWER,
> -
>_power_state, sizeof(dpcd_power_state));
> - if (status == DC_OK && dpcd_power_state ==
> DP_POWER_STATE_D0) {
> - /* blank dp stream before power off
> receiver*/
> - if (dc->links[i]->link_enc->funcs-
> >get_dig_frontend) {
> - unsigned int fe = dc->links[i]-
> >link_enc->funcs->get_dig_frontend(dc->links[i]->link_enc);
> -
> - for (j = 0; j < dc->res_pool-
> >stream_enc_count; j++) {
> - if (fe == dc-
> >res_pool->stream_enc[j]->id) {
> - dc-
> >res_pool->stream_enc[j]->funcs->dp_blank(
> -
>dc->res_pool->stream_enc[j]);
> - break;
> - }
> + /* if any of the displays are lit up turn them off */
> + status = core_link_read_dpcd(dc->links[i],
> DP_SET_POWER,
> + _power_state,
> sizeof(dpcd_power_state));
> + if (status == DC_OK && dpcd_power_state ==
> DP_POWER_STATE_D0) {
> + /* blank dp stream before power off
> receiver*/
> + if (dc->links[i]->link_enc->funcs-
> >get_dig_frontend) {
> + unsigned int fe =
> +dc->links[i]->link_enc->funcs->get_dig_frontend(dc->links[i]->link_enc)
> +;
> +
> + for (j = 0; j < dc->res_pool-
> >stream_enc_count; j++) {
> + if (fe == dc->res_pool-
> >stream_enc[j]->id) {
> + dc->res_pool-
> >stream_enc[j]->funcs->dp_blank(
> +
>dc->res_pool->stream_enc[j]);
> + break;
>}
>}
> - dp_receiver_power_ctrl(dc->links[i],
> false);
>}
> + dp_receiver_power_ctrl(dc->links[i], false);
>}
>}
>}
> --
> 2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


RE: [PATCH] drm/amd/display: Remove aconnector condition check for dpcd read

2020-04-17 Thread Liu, Zhan
+ Joseph



Hi Joseph,

Would you like to help me review this change? This was a follow-up on the 
discussion we had earlier this year.

Thanks,
Zhan


> -Original Message-
> From: Liu, Zhan 
> Sent: 2020/April/16, Thursday 3:24 PM
> To: amd-gfx@lists.freedesktop.org; Liu, Zhan 
> Subject: [PATCH] drm/amd/display: Remove aconnector condition check for
> dpcd read
> 
> [Why]
> Aconnector is not necessary to be NULL in order to read dpcd successfully.
> 
> Actually if we rely on checking aconnector here, we won't be able to turn off
> all displays before doing display detection. That will cause some MST hubs
> not able to light up.
> 
> [How]
> Remove aconnector check when turning off all displays at hardware
> initialization stage.
> 
> Signed-off-by: Zhan Liu 
> ---
>  .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 36 ---
>  1 file changed, 14 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> index 9f41efddc9bc..6f33f3f0d023 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> @@ -1332,31 +1332,23 @@ void dcn10_init_hw(struct dc *dc)
>   if (dc->links[i]->connector_signal !=
> SIGNAL_TYPE_DISPLAY_PORT)
>   continue;
> 
> - /*
> -  * core_link_read_dpcd() will invoke
> dm_helpers_dp_read_dpcd(),
> -  * which needs to read dpcd info with the help of
> aconnector.
> -  * If aconnector (dc->links[i]->prev) is NULL, then
> dpcd status
> -  * cannot be read.
> -  */
> - if (dc->links[i]->priv) {
> - /* if any of the displays are lit up turn them
> off */
> - status = core_link_read_dpcd(dc->links[i],
> DP_SET_POWER,
> -
>   _power_state, sizeof(dpcd_power_state));
> - if (status == DC_OK && dpcd_power_state ==
> DP_POWER_STATE_D0) {
> - /* blank dp stream before power off
> receiver*/
> - if (dc->links[i]->link_enc->funcs-
> >get_dig_frontend) {
> - unsigned int fe = dc->links[i]-
> >link_enc->funcs->get_dig_frontend(dc->links[i]->link_enc);
> -
> - for (j = 0; j < dc->res_pool-
> >stream_enc_count; j++) {
> - if (fe == dc-
> >res_pool->stream_enc[j]->id) {
> - dc-
> >res_pool->stream_enc[j]->funcs->dp_blank(
> -
>   dc->res_pool->stream_enc[j]);
> - break;
> - }
> + /* if any of the displays are lit up turn them off */
> + status = core_link_read_dpcd(dc->links[i],
> DP_SET_POWER,
> + _power_state,
> sizeof(dpcd_power_state));
> + if (status == DC_OK && dpcd_power_state ==
> DP_POWER_STATE_D0) {
> + /* blank dp stream before power off
> receiver*/
> + if (dc->links[i]->link_enc->funcs-
> >get_dig_frontend) {
> + unsigned int fe =
> +dc->links[i]->link_enc->funcs->get_dig_frontend(dc->links[i]->link_enc)
> +;
> +
> + for (j = 0; j < dc->res_pool-
> >stream_enc_count; j++) {
> + if (fe == dc->res_pool-
> >stream_enc[j]->id) {
> + dc->res_pool-
> >stream_enc[j]->funcs->dp_blank(
> +
>   dc->res_pool->stream_enc[j]);
> + break;
>   }
>   }
> - dp_receiver_power_ctrl(dc->links[i],
> false);
>   }
> + dp_receiver_power_ctrl(dc->links[i], false);
>   }
>   }
>   }
> --
> 2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx