On 15.04.2019 13:19, Tomi Valkeinen wrote: > On 15/04/2019 13:09, Andrzej Hajda wrote: >> On 26.03.2019 11:31, Tomi Valkeinen wrote: >>> In tc_bridge_mode_set callback, we store the pointer to the given >>> drm_display_mode, and use the mode later. Storing a pointer in such a >>> way looks very suspicious to me, and I have observed odd issues where >>> the timings were apparently (at least mostly) zero. >>> >>> Do a copy of the drm_display_mode instead to ensure we don't refer to >>> freed/modified data. >> >> Why not tc->bridge.encoder->crtc->state->adjusted_mode or >> >> tc->bridge.encoder->crtc->state->mode ? >> >> >> They should exists if the mode is set. > Well, one reason was that my main concern was to get the DP output > working (as it was quite broken wrt. the link setup), and I wanted to > minimize all the other changes. This change felt the simplest way to fix > this issue and get forward, without possibly causing new problems. > > Second, I'm a bit confused about DRM mode setting, and didn't want to > possibly move the driver into the wrong direction. It's not clear to me > whether the "mode" referred here is about the input or the output mode. > And, in both cases, if there's a bridge between the CRTC and the > TC358767, we would definitely be looking at the wrong mode if we peek at > the CRTC's.
DRM does not support multiple intermediate modes, there is .mode as requested by userspace and .adjusted_mode with twisted semantic, with recent improvements[1]. [1]: https://www.kernel.org/doc/html/latest/gpu/drm-kms.html Regards Andrzej > > Tomi > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
