[PATCH] drm/amd/display: Add dm support for DP 1.4 Compliance edid corruption test

[Zuo, Jerry]

[AMD Official Use Only - Internal Distribution Only]

The error handling for edid read right now is categorized as:
1. If base block is fetched with checksum mismatch, drm will set connector->edid_corrupt and return with Null edid buffer. This case will be handled as compliance test case 4.2.2.6, retuning EDID_BAD_CHECKSUM right away without the need to got ahead to parse edid caps.
2. If edid buffer cannot be created, return EDID_NO_RESPONSE but do not set connector->edid_corrupt
3. For extension blocks, drm returns any valid blocks. For invalid extension blocks, drm doesn't set connector->edid_corrupt. This case will be further handled by dm_helpers_parse_edid_caps() with 3 times retries. If all failed, will return EDID_BAD_CHECKSUM dc as well to enable fail-safe mode.

Regards,
Jerry

-----Original Message-----
From: Wentland, Harry <Harry.Wentland at amd.com>
Sent: May 1, 2020 10:14 AM
To: Zuo, Jerry <Jerry.Zuo at amd.com>; amd-gfx at lists.freedesktop.org; Wu, Hersen <hersenxs.wu at amd.com>; S, Shirish <Shirish.S at amd.com>
Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Siqueira, Rodrigo <Rodrigo.Siqueira at amd.com>
Subject: Re: [PATCH] drm/amd/display: Add dm support for DP 1.4 Compliance edid corruption test



On 2020-04-29 1:58 p.m., Jerry (Fangzhi) Zuo wrote:
> It works together with drm framework
> "drm: Add support for DP 1.4 Compliance edid corruption test"
>
> Signed-off-by: Jerry (Fangzhi) Zuo <Jerry.Zuo at amd.com>
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 40
> ++++++-------------
>  1 file changed, 13 insertions(+), 27 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 c407f06cd1f5..b086d5c906e0 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
> @@ -554,6 +554,7 @@ enum dc_edid_status dm_helpers_read_local_edid(
>  struct dc_sink *sink)
>  {
>  struct amdgpu_dm_connector *aconnector = link->priv;
> +struct drm_connector *connector = &aconnector->base;
>  struct i2c_adapter *ddc;
>  int retry = 3;
>  enum dc_edid_status edid_status;
> @@ -571,6 +572,15 @@ enum dc_edid_status dm_helpers_read_local_edid(
>
>  edid = drm_get_edid(&aconnector->base, ddc);
>
> +/* DP Compliance Test 4.2.2.6 */
> +if (link->aux_mode && connector->edid_corrupt)
> +drm_dp_send_real_edid_checksum(&aconnector->dm_dp_aux.aux,
> +connector->real_edid_checksum);
> +
> +if (!edid && connector->edid_corrupt) {
> +connector->edid_corrupt = false;
> +return EDID_BAD_CHECKSUM;

You return EDID_BAD_CHECKSUM here but the surrounding loop uses "edid_status == EDID_BAD_CHECKSUM" as condition to go again. Is this duplicating functionality that dm_helpers_parse_edid_caps did?

Harry

> +}
> +
>  if (!edid)
>  return EDID_NO_RESPONSE;
>
> @@ -605,34 +615,10 @@ enum dc_edid_status dm_helpers_read_local_edid(
>  DRM_ERROR("EDID err: %d, on connector: %s",
>  edid_status,
>  aconnector->base.name);
> -if (link->aux_mode) {
> -union test_request test_request = { {0} };
> -union test_response test_response = { {0} };
> -
> -dm_helpers_dp_read_dpcd(ctx,
> -link,
> -DP_TEST_REQUEST,
> -&test_request.raw,
> -sizeof(union test_request));
> -
> -if (!test_request.bits.EDID_READ)
> -return edid_status;
>
> -test_response.bits.EDID_CHECKSUM_WRITE = 1;
> -
> -dm_helpers_dp_write_dpcd(ctx,
> -link,
> -DP_TEST_EDID_CHECKSUM,
> -&sink->dc_edid.raw_edid[sink->dc_edid.length-1],
> -1);
> -
> -dm_helpers_dp_write_dpcd(ctx,
> -link,
> -DP_TEST_RESPONSE,
> -&test_response.raw,
> -sizeof(test_response));
> -
> -}
> +/* DP Compliance Test 4.2.2.3 */
> +if (link->aux_mode)
> +drm_dp_send_real_edid_checksum(&aconnector->dm_dp_aux.aux,
> +sink->dc_edid.raw_edid[sink->dc_edid.length-1]);
>
>  return edid_status;
>  }
>