On Mon, Oct 24, 2016 at 03:08:48PM -0700, Manasi Navare wrote: > On Mon, Oct 24, 2016 at 09:12:35AM +0200, Daniel Vetter wrote: > > On Mon, Oct 24, 2016 at 9:00 AM, Manasi Navare > > <manasi.d.navare@xxxxxxxxx> wrote: > > >> I guess we just need to do some additional work on top to make sure the > > >> vblank ioctl can't see invalid state. Which would then again make upfront > > >> link trainig possible ... > > Thanks for the explanation. So the atomic_commit code in the driver > calls drm_crtc_send_vblank_event(crtc, crtc->state->event);, is this > what needs to be filtered to be seen by Vblank IOCTL? > > > > > > > > Could you share some more details on what work would need to be done > > > for vblank? Then I can investigate it a little bit more tomor during the day. > > > > So the trouble is that drm_irq.c is still from the old pre-kms days. > > Which means it deals in int pipe instead of struct drm_crtc *, among > > other things. But we can't simply switch over because some old drivers > > (pre-kms ones) still depend upon that old code. Hence we need: > > > > 1. Duplicate most of the code in drm_irq.c into a new > > drm_crtc_vblank.c, which is only used for kms drivers. And in the > > ioctl code have a DRIVER_MODESET check and either call the new kms > > version (in drm_crtc_vblank.c) or the old one (still in drm_irq.c). > > What functions need to be duplicated? Figuring out the exact list is way this is painful. Since we don't need everything, but just enough to make the new drm_crtc_vblank.c work. And then as a second step we probably should move all the functions starting with drm_crtc_vblank_ which are exported to drivers over to that file too. > Which IOCTL are we talking about here? Do you mean in the atomic_commit_tail > where we check for driver modeset, is where we should then call the new kms version > of drm_crtc_send_vblank_event()? > > > > 2. Convert the int pipe into a drm_crtc pointer in the new kms code. > > For prettiness we could (try) to do that as much as possible. > > > > 3. Grab the drm_crtc->mutex in the ioctl code to block these races. > > Again which IOCTL needs to grab the drm_crtc->mutex? The vblank ioctl in drm_irq.c, implemented by drm_wait_vblank. > What is the end goal of thsi task, what race conditions are we trying to avoid > in case of these modesets during link training failures? Modeset code calls drm_crtc_vblank_on/off when doing a full modeset. That changes the vblank status, which can be observed through the vblank ioctl by userspace: In between the calls to _off/_on it will return -EINVAL, instead of the last vblank timestamp (for a query) or doing the vblank wait (otherwise), which is what it should do. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
