Re: [PATCH] drm/amd/display: fix return value of dm_dp_aux_transfer() (V2)
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 --- 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
Re: [PATCH] drm/amd/display: fix return value of dm_dp_aux_transfer() (V2)
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 --- 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? 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
Re: [PATCH] drm/amd/display: fix return value of dm_dp_aux_transfer() (V2)
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 > --- > 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. 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
[PATCH] drm/amd/display: fix return value of dm_dp_aux_transfer() (V2)
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 --- 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) + return -EIO; + else + return read_bytes; case DP_AUX_I2C_WRITE: res = dal_ddc_service_write_dpcd_data( TO_DM_AUX(aux)->ddc_service, -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx