Re: [PATCH RFC 0/7] drm/display: dp: add new DPCD access functions

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 23 Jan 2025, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote:
> On Fri, Jan 17, 2025 at 10:56:35AM +0200, Dmitry Baryshkov wrote:
>> Existing DPCD access functions return an error code or the number of
>> bytes being read / write in case of partial access. However a lot of
>> drivers either (incorrectly) ignore partial access or mishandle error
>> codes. In other cases this results in a boilerplate code which compares
>> returned value with the size.
>> 
>> As suggested by Jani implement new set of DPCD access helpers, which
>> ignore partial access, always return 0 or an error code. Reimplement
>> existing helpers using the new functions to ensure backwards
>> compatibility.
>> 
>> This series targets only the DRM helpers code. If the approach is found
>> to be acceptable, each of the drivers should be converted on its own.
>
> Gracious ping for the series, it's been posted a week ago.

It's a bit of a bummer the names become longer. I don't have a better
suggestion, though.

I do like it that the error handling becomes easier and more uniform
overall, and this actually fixes many places that only checked for < 0
with the old functions.

I glanced through the series, I didn't spot anything obviously wrong,
but didn't do a thorough review either. I can do once we have wider
buy-in for the idea in general, so the effort won't be wasted.

For now,

Acked-by: Jani Nikula <jani.nikula@intel.com>

>
>> 
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>> Dmitry Baryshkov (7):
>>       drm/display: dp: change drm_dp_dpcd_read_link_status() return
>>       drm/display: dp: implement new access helpers
>>       drm/display: dp: use new DCPD access helpers
>>       drm/display: dp-aux-dev: use new DCPD access helpers
>>       drm/display: dp-cec: use new DCPD access helpers
>>       drm/display: dp-mst-topology: use new DCPD access helpers
>>       drm/display: dp-tunnel: use new DCPD access helpers
>> 
>>  drivers/gpu/drm/display/drm_dp_aux_dev.c      |  12 +-
>>  drivers/gpu/drm/display/drm_dp_cec.c          |  37 ++-
>>  drivers/gpu/drm/display/drm_dp_helper.c       | 345 +++++++++++---------------
>>  drivers/gpu/drm/display/drm_dp_mst_topology.c | 132 +++++-----
>>  drivers/gpu/drm/display/drm_dp_tunnel.c       |  20 +-
>>  drivers/gpu/drm/msm/dp/dp_ctrl.c              |  45 ++--
>>  drivers/gpu/drm/msm/dp/dp_link.c              |  17 +-
>>  include/drm/display/drm_dp_helper.h           |  81 +++++-
>>  include/drm/display/drm_dp_mst_helper.h       |  10 +-
>>  9 files changed, 354 insertions(+), 345 deletions(-)
>> ---
>> base-commit: 440aaf479c9aaf5ecea9a463eb826ec243d5f1cf
>> change-id: 20241231-drm-rework-dpcd-access-b0fc2e47d613
>> 
>> Best regards,
>> -- 
>> Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> 

-- 
Jani Nikula, Intel