Hi! a couple people poked me to take a look at this, hopefully I can provide some helpful insight here. So: I've tried a _lot_ of various redesigns with MST and some portions of the DP helpers. I actually was going to try to write up some common helpers for handling link training across drivers, but when I started trying to implement them (ironically, I think it was in i915!) I realized I wasn't getting rid of nearly as much code as I thought I was going to. Now-I'd love to tell you if this series is good or bad, but I'm not entirely sure myself either. Sometimes I wonder myself if I'm overcomplicating things with MST. The only way I've really found of figuring out whether or not helpers like this are overkill is to just implement them everywhere. With my atomic VCPI helpers for MST, I tried implementing them everywhere I could (except for amdgpu, but I _did_ try there originally!) to see how awkward they were to use. I think if there's questions to how useful this series is, it'd probably be good to try implementing these helpers in drivers like i915 to see how things play out. It should also be noted that I did actually try to come up with common link rate helpers like this one, but I ended up realizing I was adding far more code then I was getting rid of after I tried implementing them in i915 (ironic, huh?). Things got even more complicated when I looked at how nouveau/nvidia hardware does link training. On Fri, 2019-09-20 at 18:00 +0200, Thierry Reding wrote: > On Mon, Sep 02, 2019 at 01:31:00PM +0200, Thierry Reding wrote: > > From: Thierry Reding <treding@xxxxxxxxxx> > > > > Hi, > > > > this series of patches improves the DP helpers a bit and cleans up some > > inconsistencies along the way. > > > > v2 incorporates all review comments add collects Reviewed-bys from v1. > > > > Thierry > > > > Thierry Reding (21): > > drm/dp: Sort includes alphabetically > > drm/dp: Add missing kerneldoc for struct drm_dp_link > > drm/dp: Add drm_dp_link_reset() implementation > > drm/dp: Track link capabilities alongside settings > > drm/dp: Turn link capabilities into booleans > > drm/dp: Probe link using existing parsing helpers > > drm/dp: Read fast training capability from link > > drm/dp: Read TPS3 capability from sink > > drm/dp: Read channel coding capability from sink > > drm/dp: Read alternate scrambler reset capability from sink > > drm/dp: Read eDP version from DPCD > > drm/dp: Read AUX read interval from DPCD > > drm/dp: Do not busy-loop during link training > > drm/dp: Use drm_dp_aux_rd_interval() > > drm/dp: Add helper to get post-cursor adjustments > > drm/dp: Set channel coding on link configuration > > drm/dp: Enable alternate scrambler reset when supported > > drm/dp: Add drm_dp_link_choose() helper > > drm/dp: Add support for eDP link rates > > drm/dp: Remove a gratuituous blank line > > drm/bridge: tc358767: Use DP nomenclature > > Anyone interested in reviewing these? > > Thierry > > > drivers/gpu/drm/bridge/tc358767.c | 22 +- > > drivers/gpu/drm/drm_dp_helper.c | 327 ++++++++++++++++++++++--- > > drivers/gpu/drm/msm/edp/edp_ctrl.c | 12 +- > > drivers/gpu/drm/rockchip/cdn-dp-core.c | 8 +- > > drivers/gpu/drm/rockchip/cdn-dp-reg.c | 13 +- > > drivers/gpu/drm/tegra/dpaux.c | 8 +- > > drivers/gpu/drm/tegra/sor.c | 32 +-- > > include/drm/drm_dp_helper.h | 124 +++++++++- > > 8 files changed, 459 insertions(+), 87 deletions(-) > > > > -- > > 2.22.0 > > > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Cheers, Lyude Paul _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
