On 2020-05-04 10:56 a.m., Zuo, Jerry wrote:
> [AMD Official Use Only - Internal Distribution Only]
>
>
> 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@xxxxxxx>
>> ---
>> .../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
>
>
> It is not duplicate. You mentioned dm_helpers_parse_edid_caps() with
> retries is fallen into the 3rd checkup step below:
> 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 going ahead to parse edid caps --> newly added
> 2. If edid buffer cannot be created, return EDID_NO_RESPONSE but do not
> set connector->edid_corrupt
> 3. 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 failed any step above with EDID_BAD_CHECKSUM, will enable fail-safe
> mode in DC.
>
Thanks. Just curious if there's a way to simplify the code.
Either way, this patch is
Reviewed-by: Harry Wentland <harry.wentland@xxxxxxx>
Harry
> Jerry
>
>
>> + }
>> +
>> 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;
>> }
>>
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
