[Intel-gfx] [PATCH v2 2/2] Test case for drm_vblank_cleanup refcount validation patch

[Daniel Vetter]

On Tue, Oct 31, 2017 at 08:37:21PM +0530, PrasannaKumar Muralidharan wrote:
> Hi Daniel,
> 
> On 31 October 2017 at 15:51, Daniel Vetter <daniel at ffwll.ch> wrote:
> >
> > On Mon, Oct 30, 2017 at 06:01:12PM +0530, PrasannaKumar Muralidharan wrote:
> > > Hi Daniel,
> > >
> > > On 30 October 2017 at 15:40, Daniel Vetter <daniel at ffwll.ch> wrote:
> > > > On Wed, Oct 25, 2017 at 08:44:45PM +0530, PrasannaKumar Muralidharan wrote:
> > > >> Hi All,
> > > >>
> > > >> On 24 October 2017 at 22:18, PrasannaKumar Muralidharan
> > > >> <prasannatsmkumar at gmail.com> wrote:
> > > >> > In i915 driver unload drm_vblank_get is added to test whether
> > > >> > drm_vblank_cleanup refcount validation patch is working.
> > > >> >
> > > >> > Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar at gmail.com>
> > > >> > ---
> > > >> > Changes in v2:
> > > >> > Use drm_crtc_vblank_get instead of _put. In previous patch _put was wrongly
> > > >> > used.
> > > >> >
> > > >> >  drivers/gpu/drm/i915/i915_drv.c | 7 +++++++
> > > >> >  1 file changed, 7 insertions(+)
> > > >> >
> > > >> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > >> > index 9f45cfe..4aee1c0 100644
> > > >> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > >> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > >> > @@ -1373,6 +1373,13 @@ void i915_driver_unload(struct drm_device *dev)
> > > >> >         struct drm_i915_private *dev_priv = to_i915(dev);
> > > >> >         struct pci_dev *pdev = dev_priv->drm.pdev;
> > > >> >
> > > >> > +       enum pipe pipe;
> > > >> > +       for_each_pipe(dev_priv, pipe) {
> > > >> > +               struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv,
> > > >> > +                                                                 pipe);
> > > >> > +               drm_crtc_vblank_get(&crtc->base);
> > > >> > +       }
> > > >> > +
> > > >> >         i915_driver_unregister(dev_priv);
> > > >> >
> > > >> >         if (i915_gem_suspend(dev_priv))
> > > >> > --
> > > >> > 2.10.0
> > > >> >
> > > >>
> > > >> From https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6167/fi-ilk-650/igt@drv_module_reload@basic-reload.html
> > > >> it can be seen that this patch triggers warning when vblank->refcount
> > > >> > 0 in vblank cleanup. This tests patch 1 successfully.
> > > >>
> > > >> I think patch 1 is good to go.
> > > >
> > > > Yeah it works, but we don't know whether it breaks anything yet. Can you
> > > > pls resubmit just the first patch? Abusing CI was just an idea to get it
> > > > fully tested, before we can merge it still needs to pass full CI. We just
> > > > had an issue 2 weeks ago where CI blew up because an untested patch landed
> > > > that broke every test :-/
> > > > -Daniel
> > > > --
> > > > Daniel Vetter
> > > > Software Engineer, Intel Corporation
> > > > http://blog.ffwll.ch
> > >
> > > I have already sent that patch alone. Please look at
> > > https://patchwork.freedesktop.org/patch/184866/.
> >
> > Seems to fail in CI:
> >
> > https://patchwork.freedesktop.org/series/32648/
> >
> > I guess you need to test this on a local box somewhere to figure out
> > what's wrong.
> 
> My patch is supposed to catch problem with drivers. It warns when
> vblank refcount is non-zero in drm_vblank_cleanup call. From CI log it
> can be seen that warning being triggered. I feel that my patch is
> exposing existing issue.
> 
> If I misinterpreted something please let me know.

This is probably what's happening, but I can't merge a patch that breaks
CI. So we need to track down that issue before merging.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch