Hi Andrzej, On Mon, Apr 15, 2019 at 02:12:46PM +0200, Andrzej Hajda wrote: > 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, *yet* :-) I don't know when we will need intermediate modes and bridge states, but we'll get there. I think this patch is safe, and I agree with Tomi that it minimizes the impact on the driver. > 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, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
