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

[PrasannaKumar Muralidharan]

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.

Thanks,
PrasannaKumar