Re: [PATCH] drm/amd/display: fix return value of dm_dp_aux_transfer() (V2)

["S, Shirish" <sshankar-5C7GfCeVMHo@public.gmane.org>]

On 4/23/2018 9:53 AM, S, Shirish wrote:
>
>
> On 4/20/2018 11:52 PM, Harry Wentland wrote:
>> On 2018-04-17 10:56 PM, Shirish S wrote:
>>> Currently the dm_dp_aux_transfer() does not parse
>>> the return value of dal_ddc_service_read_dpcd_data(), which also
>>> has a failure case.
>>> This patch captures the same and ensures the i2c operation status is
>>> sent appropriately to the drm framework.
>>>
>>> V2: Updated commit message.
>>>
>>> Signed-off-by: Shirish S <shirish.s@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 6 
>>> +++++-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git 
>>> a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c 
>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>>> index 782491e..7ac124d 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>>> @@ -115,7 +115,11 @@ static ssize_t dm_dp_aux_transfer(struct 
>>> drm_dp_aux *aux,
>>>                   msg->address,
>>>                   msg->buffer,
>>>                   msg->size);
>>> -        return read_bytes;
>>> +        if (read_bytes != msg->size &&
>>> +            read_bytes >= DDC_RESULT_FAILED_OPERATION)
>> This doesn't look right. We shouldn't be returning the size or error 
>> code from the same function. This will not work if we submit a read 7 
>> or 8 bytes and get an error.
> Agree,  but hope you understood the issue, to re-iterate, in case of 
> failure we reuturn a +ve number(7 or 8) back from dm_dp_aux_transfer() 
> leading to edid read failures later.
> I have 2 suggestions:
> 1. change the "enum ddc_result {" to #defines of -ve error codes? or
> 2. make enum start with 129 (> than the max read_bytes possible)?
> let me know which one would be better?
Any suggestions?
> Thanks,
> Regards,
> Shirish S
>>
>> Harry
>>
>>> +            return -EIO;
>>> +        else
>>> +            return read_bytes;
>>>       case DP_AUX_I2C_WRITE:
>>>           res = dal_ddc_service_write_dpcd_data(
>>>                   TO_DM_AUX(aux)->ddc_service,
>>>
>

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