| Message ID | 20250301-drm-rework-dpcd-access-v2-0-4d92602fc7cd@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/display: dp: add new DPCD access functions | expand |
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,
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,
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.
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,