On Wed, Dec 26, 2018 at 09:50:44AM -0800, Deepak Singh Rawat wrote: > On Tue, 2018-12-25 at 07:01 -0800, Thomas Hellstrom wrote: > > On Mon, 2018-12-24 at 11:49 +0100, Daniel Vetter wrote: > > > On Fri, Dec 21, 2018 at 8:56 PM Thomas Hellstrom < > > > thellstrom@xxxxxxxxxx> wrote: > > > > Reviewed-by: Thomas Hellstrom <thellstrom@xxxxxxxxxx> > > > > > > > > Daniel, Dave, could you help try to get this patch in -next > > > > before > > > > the > > > > merge window. Otherwise people will start to experience random > > > > kernel > > > > crashes. I figure we don't want to wait until first -fixes pull > > > > with > > > > this. > > > > > > Afaiui this will only blow up with new userspace on new kernels, > > > that > > > doesn't feel like rushing an updated -next out the door material to > > > me. > > > > Hmm, this was discovered when the drm-read igt test hit an OOPS, > > I guess when damage_clips is unintentionally Non-Null. > > > > > > > More concerning is why this fell through the cracks: > > > > I agree. Sinc I'm on vacation a very quick answer. Will investigate > > more thogroughly when we get back. > > > > > - We have an igt, do those testcases not hit this bug? > > > > The drm-read igt intermittently hit the bug. > > > > > > > - Were the tests not run before you've sent out the pull? > > > > No, and I guess that't the main culprit. The igt doesn't work well > > with > > vmwgfx, mostly because many tests are assuming gem functionality, > > there > > fore we doesn't run them regularly, but mostly a pre-commit testsuite > > that focuses more on rendering correctness than modesetting > > functionality. That said, Deepak has over time put quite an effort > > into > > making at least part of that test suite run well with vmwgfx, but I > > didn't consider it for this pull request. Will make sure we do in the > > future for core drm changes. > > > > > - The merged version doesn't seem to match any of the versions I've > > > found on dri-devel, I guess should have been resend when there was > > > conflicts? > > > > Yes, I think there was at least one trivial merge conflict and a > > couple > > of minor checkpatch.pl fixups in the pull request. Typically for > > vmwgfx > > we don't resend the resulting patches if there are minor changes like > > that and they are reviewed internally. Need to get back to you about > > exactly where in the process we failed here. > > I did ran igt after rebasing with upstream drm-next but only kms tests > and didn't saw any problem. We have glretrace running continuously with > rebooting VM's every time it runs and it too didn't failed so didn't > suspected anything wrong with what I sent. Unfortunately we don't run > igt continuously so I guess that is what we should work on. > > I am yet to investigate why only drm_read failed. Although from stack > trace I can see it is failing when set_config is called from vmwgfx > fb_dev driver. Yeah, drm_read failing isn't really the testcase I expected to blow up at all here. Sounds like an intriguing story at least on what exactly is going wrong here ... -Daniel > > > > > Thanks, > > Thomas > > > > > > > > > - Some other crack in the matrix? > > > > > > > Thanks, > > > > Thomas > > > > > > > > > > > > On Fri, 2018-12-21 at 11:35 -0800, Deepak Rawat wrote: > > > > > Somehow the code to put the damage blob on destroy plane state > > > > > and > > > > > set > > > > > the blob to NULL when duplicate plane state was not merged. May > > > > > be > > > > > because the files are refactored since the patch was written. > > > > > With > > > > > this > > > > > fix add those. > > > > > > > > > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > > > > > Signed-off-by: Deepak Rawat <drawat@xxxxxxxxxx> > > > > > > Needs a Fixes: tag referencing the broken commit, pls remember to > > > add > > > these anytime you fix an issue with a commit. I'll add that and > > > push > > > it to drm-misc-next-fixes. > > Thanks Daniel for this. Will make sure in future. > > > > > > > Thanks, Daniel > > > > > > > > --- > > > > > drivers/gpu/drm/drm_atomic_state_helper.c | 3 +++ > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c > > > > > b/drivers/gpu/drm/drm_atomic_state_helper.c > > > > > index 3ba996069d69..709355c6bac6 100644 > > > > > --- a/drivers/gpu/drm/drm_atomic_state_helper.c > > > > > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c > > > > > @@ -241,6 +241,7 @@ void > > > > > __drm_atomic_helper_plane_duplicate_state(struct drm_plane > > > > > *plane, > > > > > > > > > > state->fence = NULL; > > > > > state->commit = NULL; > > > > > + state->fb_damage_clips = NULL; > > > > > } > > > > > EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state); > > > > > > > > > > @@ -285,6 +286,8 @@ void > > > > > __drm_atomic_helper_plane_destroy_state(struct drm_plane_state > > > > > *state) > > > > > > > > > > if (state->commit) > > > > > drm_crtc_commit_put(state->commit); > > > > > + > > > > > + drm_property_blob_put(state->fb_damage_clips); > > > > > } > > > > > EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state); > > > > > > > > > > > > > > -- > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > +41 (0) 79 365 57 48 - > > > > https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ffwll.ch&data=02%7C01%7Cthellstrom%40vmware.com%7Ca62b8426087f410a689b08d6698d9110%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636812454080233380&sdata=6CZm4TU0A7wLpd1sXOUA3URmMQJqndy1WHxhtjnU8P8%3D&reserved=0 > -- 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
