[PATCH] drm: Put damage blob when destroy plane state

[Thomas Hellstrom]

On Mon, 2018-12-24 at 11:49 +0100, Daniel Vetter wrote:
> On Fri, Dec 21, 2018 at 8:56 PM Thomas Hellstrom <
> thellstrom at vmware.com> wrote:
> > Reviewed-by: Thomas Hellstrom <thellstrom at vmware.com>
> > 
> > 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.

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 at ffwll.ch>
> > > Signed-off-by: Deepak Rawat <drawat at vmware.com>
> 
> 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
> 
> > > ---
> > >  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