Dmitry Baryshkov
2024-Jul-16 16:35 UTC
DisplayPort: handling of HPD events / link training
On Tue, 16 Jul 2024 at 18:58, Thomas Zimmermann <tzimmermann at suse.de> wrote:> > Hi > > Am 27.02.24 um 23:40 schrieb Dmitry Baryshkov: > > Hello, > > > > We are currently looking at checking and/or possibly redesigning the > > way the MSM DRM driver handles the HPD events and link training. > > > > After a quick glance at the drivers implementing DP support, I noticed > > following main approaches: > > - Perform link training at the atomic_enable time, don't report > > failures (mtk, analogix, zynqmp, tegra, nouveau) > > - Perform link training at the atomic_enable time, report errors using > > link_status property (i915, mhdp8546) > > - Perform link training on the plug event (msm, it8605). > > - Perform link training from the DPMS handler, also calling it from > > the enable callback (AMDGPU, radeon). > > > > It looks like the majority wins and we should move HPD to > > atomic_enable time. Is that assumption correct? > > Did you ever receive an answer to this question? I currently investigate > ast's DP code, which does link training as part of detecting the > connector state (in detect_ctx). But most other drivers do this in > atomic_enable. I wonder if ast should follow.Short answer: yes, the only proper place to do it is atomic_enable(). Long answer: I don't see a way to retrigger link training in ast_dp.c Without such change you are just shifting things around. The end-result of moving link-training to atomic_enable() is that each enable can trigger link training, possibly lowering the link rate, etc. if link training is just a status bit from the firmware that we don't control, it doesn't make real-real sense to move it.> > Best regards > Thomas > > > > > Also two related questions: > > - Is there a plan to actually make use of the link_status property? > > Intel presented it at FOSDEM 2018, but since that time it was not > > picked up by other drivers. > > > > - Is there any plan to create generic DP link training helpers? After > > glancing through the DP drivers there is a lot of similar code in the > > link training functions, with minor differences here and there. And > > it's those minor differences that bug me. It means that drivers might > > respond differently to similar devices. Or that there might be minor > > bugs here and there. > > > > -- > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Frankenstrasse 146, 90461 Nuernberg, Germany > GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman > HRB 36809 (AG Nuernberg) >-- With best wishes Dmitry
Thomas Zimmermann
2024-Jul-16 16:48 UTC
DisplayPort: handling of HPD events / link training
Hi Am 16.07.24 um 18:35 schrieb Dmitry Baryshkov:> On Tue, 16 Jul 2024 at 18:58, Thomas Zimmermann <tzimmermann at suse.de> wrote: >> Hi >> >> Am 27.02.24 um 23:40 schrieb Dmitry Baryshkov: >>> Hello, >>> >>> We are currently looking at checking and/or possibly redesigning the >>> way the MSM DRM driver handles the HPD events and link training. >>> >>> After a quick glance at the drivers implementing DP support, I noticed >>> following main approaches: >>> - Perform link training at the atomic_enable time, don't report >>> failures (mtk, analogix, zynqmp, tegra, nouveau) >>> - Perform link training at the atomic_enable time, report errors using >>> link_status property (i915, mhdp8546) >>> - Perform link training on the plug event (msm, it8605). >>> - Perform link training from the DPMS handler, also calling it from >>> the enable callback (AMDGPU, radeon). >>> >>> It looks like the majority wins and we should move HPD to >>> atomic_enable time. Is that assumption correct? >> Did you ever receive an answer to this question? I currently investigate >> ast's DP code, which does link training as part of detecting the >> connector state (in detect_ctx). But most other drivers do this in >> atomic_enable. I wonder if ast should follow. > Short answer: yes, the only proper place to do it is atomic_enable().Thanks.> > Long answer: I don't see a way to retrigger link training in ast_dp.c > Without such change you are just shifting things around. The > end-result of moving link-training to atomic_enable() is that each > enable can trigger link training, possibly lowering the link rate, > etc. if link training is just a status bit from the firmware that we > don't control, it doesn't make real-real sense to move it.I have to think about what to do. People tend to copy existing drivers, which alone might be a good argument for using atomic_enable. The link training is indeed just a flag that is set by the firmware. I think it's possible to re-trigger training by powering the port down and up again. atomic_enable could likely do that. The hardware is also somewhat buggy and not fully standard conformant. Best regards Thomas> >> Best regards >> Thomas >> >>> Also two related questions: >>> - Is there a plan to actually make use of the link_status property? >>> Intel presented it at FOSDEM 2018, but since that time it was not >>> picked up by other drivers. >>> >>> - Is there any plan to create generic DP link training helpers? After >>> glancing through the DP drivers there is a lot of similar code in the >>> link training functions, with minor differences here and there. And >>> it's those minor differences that bug me. It means that drivers might >>> respond differently to similar devices. Or that there might be minor >>> bugs here and there. >>> >> -- >> -- >> Thomas Zimmermann >> Graphics Driver Developer >> SUSE Software Solutions Germany GmbH >> Frankenstrasse 146, 90461 Nuernberg, Germany >> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman >> HRB 36809 (AG Nuernberg) >> >-- -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)