On Wed, Jul 8, 2020 at 11:17 AM Jyri Sarha <jsarha@xxxxxx> wrote: > > On 07/07/2020 23:12, Daniel Vetter wrote: > > Gives us proper nonblocking support for free, and a pile of other > > things. The tilcdc code is simply old enough that it was never > > converted over, but was stuck forever with the copypasta from when it > > was initially merged. > > > > The riskiest thing with this conversion is maybe that there's an issue > > with the vblank handling or vblank event handling, which will upset > > the modern commit support in atomic helpers. But from a cursory review > > drm_crtc_vblank_on/off is called in the right places, and the event > > handling also seems to exist (albeit with much hand-rolling and > > probably some races, could perhaps be converted over to > > drm_crtc_arm_vblank_event without any real loss). > > > > Motivated by me not having to hand-roll the dma-fence annotations for > > this. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > > Cc: Jyri Sarha <jsarha@xxxxxx> > > Cc: Tomi Valkeinen <tomi.valkeinen@xxxxxx> > > I tried this out, but it is not working. Something breaks in the event > handling and event reference counting. Unfortunately my vacation is > pressing on, and I am not sure if I have time to debug the issue further > before that. Thanks a lot for testing, looks like tilcdc doesn't quite handle the event stuff in all cases, which results in the direct warning in hw_done, and then the refcount fallout in plane_destry_state (I think at least, not entirely sure about whether that's really just collateral damage or a 2nd bug). I'll try to come up with something, enjoy your vacations meanwhile! Cheers, Daniel > Anyway, I have attached the boot log with the following WARN dumps: > ---------------------------------------------------------------- > [ 12.203874] WARNING: CPU: 0 PID: 208 at > drivers/gpu/drm/drm_atomic_helper.c:2329 > drm_atomic_helper_commit_hw_done+0x144/0x168 [drm_kms_helper] > > [ 12.217682] WARNING: CPU: 0 PID: 208 at > drivers/gpu/drm/drm_atomic_helper.c:2329 > drm_atomic_helper_commit_hw_done+0x144/0x168 [drm_kms_helper] > > [ 232.156231] WARNING: CPU: 0 PID: 1315 at > drivers/gpu/drm/drm_atomic_helper.c:2329 > drm_atomic_helper_commit_hw_done+0x144/0x168 [drm_kms_helper] > > [ 232.472068] WARNING: CPU: 0 PID: 1315 at lib/refcount.c:28 > __drm_atomic_helper_plane_destroy_state+0xd0/0xe0 [drm_kms_helper] > > [ 240.611129] WARNING: CPU: 0 PID: 1317 at > drivers/gpu/drm/drm_atomic_helper.c:2329 > drm_atomic_helper_commit_hw_done+0x144/0x168 [drm_kms_helper] > ---------------------------------------------------------------- > > The first two came at boot time when setting up the fbconsole, the ones > after that came when I tried to use kmstest[1]. The fbconsole came up, > but nothing after that works. > > I am back from vacation in the beginning of august, so there may be some > time before I can debug this further. > > Best regards, > Jyri > > > > [1] https://github.com/tomba/kmsxx > > > --- > > drivers/gpu/drm/tilcdc/tilcdc_drv.c | 47 +---------------------------- > > 1 file changed, 1 insertion(+), 46 deletions(-) > > > > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c > > index 0d74a6443263..4f5fc3e87383 100644 > > --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c > > +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c > > @@ -87,55 +87,10 @@ static int tilcdc_atomic_check(struct drm_device *dev, > > return ret; > > } > > > > -static int tilcdc_commit(struct drm_device *dev, > > - struct drm_atomic_state *state, > > - bool async) > > -{ > > - int ret; > > - > > - ret = drm_atomic_helper_prepare_planes(dev, state); > > - if (ret) > > - return ret; > > - > > - ret = drm_atomic_helper_swap_state(state, true); > > - if (ret) { > > - drm_atomic_helper_cleanup_planes(dev, state); > > - return ret; > > - } > > - > > - /* > > - * Everything below can be run asynchronously without the need to grab > > - * any modeset locks at all under one condition: It must be guaranteed > > - * that the asynchronous work has either been cancelled (if the driver > > - * supports it, which at least requires that the framebuffers get > > - * cleaned up with drm_atomic_helper_cleanup_planes()) or completed > > - * before the new state gets committed on the software side with > > - * drm_atomic_helper_swap_state(). > > - * > > - * This scheme allows new atomic state updates to be prepared and > > - * checked in parallel to the asynchronous completion of the previous > > - * update. Which is important since compositors need to figure out the > > - * composition of the next frame right after having submitted the > > - * current layout. > > - */ > > - > > - drm_atomic_helper_commit_modeset_disables(dev, state); > > - > > - drm_atomic_helper_commit_planes(dev, state, 0); > > - > > - drm_atomic_helper_commit_modeset_enables(dev, state); > > - > > - drm_atomic_helper_wait_for_vblanks(dev, state); > > - > > - drm_atomic_helper_cleanup_planes(dev, state); > > - > > - return 0; > > -} > > - > > static const struct drm_mode_config_funcs mode_config_funcs = { > > .fb_create = drm_gem_fb_create, > > .atomic_check = tilcdc_atomic_check, > > - .atomic_commit = tilcdc_commit, > > + .atomic_commit = drm_atomic_helper_commit, > > }; > > > > static void modeset_init(struct drm_device *dev) > > > > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki -- 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
