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

Dmitry Baryshkov posted 7 patches 9 months, 2 weeks ago
There is a newer version of this series
drivers/gpu/drm/amd/amdgpu/atombios_dp.c           |   8 +-
.../gpu/drm/bridge/cadence/cdns-mhdp8546-core.c    |   2 +-
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            | 307 +++++++++------------
drivers/gpu/drm/display/drm_dp_mst_topology.c      | 105 ++++---
drivers/gpu/drm/display/drm_dp_tunnel.c            |  20 +-
drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c       |   4 +-
drivers/gpu/drm/msm/dp/dp_ctrl.c                   |  24 +-
drivers/gpu/drm/msm/dp/dp_link.c                   |  18 +-
drivers/gpu/drm/radeon/atombios_dp.c               |   8 +-
include/drm/display/drm_dp_helper.h                |  92 +++++-
12 files changed, 322 insertions(+), 315 deletions(-)
[PATCH RFC v2 0/7] drm/display: dp: add new DPCD access functions
Posted by Dmitry Baryshkov 9 months, 2 weeks ago
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.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
Changes in v2:
- Reimplemented new helpers using old ones (Lyude)
- Reworked the drm_dp_dpcd_read_link_status() patch (Lyude)
- Dropped the dp-aux-dev patch (Jani)
- Link to v1: https://lore.kernel.org/r/20250117-drm-rework-dpcd-access-v1-0-7fc020e04dbc@linaro.org

---
Dmitry Baryshkov (7):
      drm/display: dp: implement new access helpers
      drm/display: dp: change drm_dp_dpcd_read_link_status() return value
      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/amd/amdgpu/atombios_dp.c           |   8 +-
 .../gpu/drm/bridge/cadence/cdns-mhdp8546-core.c    |   2 +-
 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            | 307 +++++++++------------
 drivers/gpu/drm/display/drm_dp_mst_topology.c      | 105 ++++---
 drivers/gpu/drm/display/drm_dp_tunnel.c            |  20 +-
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c       |   4 +-
 drivers/gpu/drm/msm/dp/dp_ctrl.c                   |  24 +-
 drivers/gpu/drm/msm/dp/dp_link.c                   |  18 +-
 drivers/gpu/drm/radeon/atombios_dp.c               |   8 +-
 include/drm/display/drm_dp_helper.h                |  92 +++++-
 12 files changed, 322 insertions(+), 315 deletions(-)
---
base-commit: c0eb65494e59d9834af7cbad983629e9017b25a1
change-id: 20241231-drm-rework-dpcd-access-b0fc2e47d613

Best regards,
-- 
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Re: [PATCH RFC v2 0/7] drm/display: dp: add new DPCD access functions
Posted by Jani Nikula 9 months, 2 weeks ago
On Sat, 01 Mar 2025, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 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.

I think that description is for earlier versions of the series, it's the
other way round now.

Regardless, glanced through the series quickly, I like it, this is

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


>
> 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.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> Changes in v2:
> - Reimplemented new helpers using old ones (Lyude)
> - Reworked the drm_dp_dpcd_read_link_status() patch (Lyude)
> - Dropped the dp-aux-dev patch (Jani)
> - Link to v1: https://lore.kernel.org/r/20250117-drm-rework-dpcd-access-v1-0-7fc020e04dbc@linaro.org
>
> ---
> Dmitry Baryshkov (7):
>       drm/display: dp: implement new access helpers
>       drm/display: dp: change drm_dp_dpcd_read_link_status() return value
>       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/amd/amdgpu/atombios_dp.c           |   8 +-
>  .../gpu/drm/bridge/cadence/cdns-mhdp8546-core.c    |   2 +-
>  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            | 307 +++++++++------------
>  drivers/gpu/drm/display/drm_dp_mst_topology.c      | 105 ++++---
>  drivers/gpu/drm/display/drm_dp_tunnel.c            |  20 +-
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c       |   4 +-
>  drivers/gpu/drm/msm/dp/dp_ctrl.c                   |  24 +-
>  drivers/gpu/drm/msm/dp/dp_link.c                   |  18 +-
>  drivers/gpu/drm/radeon/atombios_dp.c               |   8 +-
>  include/drm/display/drm_dp_helper.h                |  92 +++++-
>  12 files changed, 322 insertions(+), 315 deletions(-)
> ---
> base-commit: c0eb65494e59d9834af7cbad983629e9017b25a1
> change-id: 20241231-drm-rework-dpcd-access-b0fc2e47d613
>
> Best regards,

-- 
Jani Nikula, Intel
Re: [PATCH RFC v2 0/7] drm/display: dp: add new DPCD access functions
Posted by Jani Nikula 9 months, 2 weeks ago
On Thu, 06 Mar 2025, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Sat, 01 Mar 2025, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 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.
>
> I think that description is for earlier versions of the series, it's the
> other way round now.
>
> Regardless, glanced through the series quickly, I like it, this is
>
> Acked-by: Jani Nikula <jani.nikula@intel.com>

PS. If you need to send another round, please Cc: intel-gfx and intel-xe
to run this through CI for both i915 and xe drivers. Thanks!

>
>
>>
>> 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.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>> Changes in v2:
>> - Reimplemented new helpers using old ones (Lyude)
>> - Reworked the drm_dp_dpcd_read_link_status() patch (Lyude)
>> - Dropped the dp-aux-dev patch (Jani)
>> - Link to v1: https://lore.kernel.org/r/20250117-drm-rework-dpcd-access-v1-0-7fc020e04dbc@linaro.org
>>
>> ---
>> Dmitry Baryshkov (7):
>>       drm/display: dp: implement new access helpers
>>       drm/display: dp: change drm_dp_dpcd_read_link_status() return value
>>       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/amd/amdgpu/atombios_dp.c           |   8 +-
>>  .../gpu/drm/bridge/cadence/cdns-mhdp8546-core.c    |   2 +-
>>  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            | 307 +++++++++------------
>>  drivers/gpu/drm/display/drm_dp_mst_topology.c      | 105 ++++---
>>  drivers/gpu/drm/display/drm_dp_tunnel.c            |  20 +-
>>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c       |   4 +-
>>  drivers/gpu/drm/msm/dp/dp_ctrl.c                   |  24 +-
>>  drivers/gpu/drm/msm/dp/dp_link.c                   |  18 +-
>>  drivers/gpu/drm/radeon/atombios_dp.c               |   8 +-
>>  include/drm/display/drm_dp_helper.h                |  92 +++++-
>>  12 files changed, 322 insertions(+), 315 deletions(-)
>> ---
>> base-commit: c0eb65494e59d9834af7cbad983629e9017b25a1
>> change-id: 20241231-drm-rework-dpcd-access-b0fc2e47d613
>>
>> Best regards,

-- 
Jani Nikula, Intel
Re: [PATCH RFC v2 0/7] drm/display: dp: add new DPCD access functions
Posted by Dmitry Baryshkov 9 months, 2 weeks ago
On Thu, Mar 06, 2025 at 03:04:50PM +0200, Jani Nikula wrote:
> On Thu, 06 Mar 2025, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > On Sat, 01 Mar 2025, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 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.
> >
> > I think that description is for earlier versions of the series, it's the
> > other way round now.
> >
> > Regardless, glanced through the series quickly, I like it, this is
> >
> > Acked-by: Jani Nikula <jani.nikula@intel.com>
> 
> PS. If you need to send another round, please Cc: intel-gfx and intel-xe
> to run this through CI for both i915 and xe drivers. Thanks!

It might be a good idea to do that anyway. I'll send next iteration,
fixing the cover letter and adding both MLs. I hope we can tolerate a
bit of spam for the sake of getting the series a proper CI test.


-- 
With best wishes
Dmitry