[Intel-gfx] [PATCH 2/4] drm/i915/selftests: align more to real device lifetimes

[Matthew Auld]

On Fri, 18 Sep 2020 at 19:22, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
>
> On Fri, Sep 18, 2020 at 7:50 PM Matthew Auld
> <matthew.william.auld at gmail.com> wrote:
> >
> > On Fri, 18 Sep 2020 at 14:25, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> > >
> > > The big change is device_add so that device_del can auto-cleanup
> > > devres resources. This allows us to use devm_drm_dev_alloc, which
> > > removes the last user of drm_dev_init.
> > >
> > > v2: Rebased
> > >
> > > v3: use devres_open/release_group so we can use devm without real
> > > hacks in the driver core or having to create an entire fake bus for
> > > testing drivers. Might want to extract this into helpers eventually,
> > > maybe as a mock_drm_dev_alloc or test_drm_dev_alloc.
> > >
> > > v4:
> > > - Fix IS_ERR handling (Matt)
> > > - Delete surplus put_device() in mock_device_release (intel-gfx-ci)
> > >
> > > Cc: Matthew Auld <matthew.william.auld at gmail.com>
> > > Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com> (v3)
> > > Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> > > ---
> > >  .../gpu/drm/i915/selftests/mock_gem_device.c  | 44 +++++++++++--------
> > >  1 file changed, 25 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > > index ac600d395c8f..816f9af15fb3 100644
> > > --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > > +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > > @@ -79,8 +79,6 @@ static void mock_device_release(struct drm_device *dev)
> > >
> > >  out:
> > >         i915_params_free(&i915->params);
> > > -       put_device(&i915->drm.pdev->dev);
> > > -       i915->drm.pdev = NULL;
> > >  }
> > >
> > >  static struct drm_driver mock_driver = {
> > > @@ -128,12 +126,6 @@ struct drm_i915_private *mock_gem_device(void)
> > >         pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
> > >         if (!pdev)
> > >                 return NULL;
> > > -       i915 = kzalloc(sizeof(*i915), GFP_KERNEL);
> > > -       if (!i915) {
> > > -               kfree(pdev);
> > > -               return NULL;
> > > -       }
> > > -
> > >         device_initialize(&pdev->dev);
> > >         pdev->class = PCI_BASE_CLASS_DISPLAY << 16;
> > >         pdev->dev.release = release_dev;
> > > @@ -144,8 +136,29 @@ struct drm_i915_private *mock_gem_device(void)
> > >         /* HACK to disable iommu for the fake device; force identity mapping */
> > >         pdev->dev.iommu = &fake_iommu;
> > >  #endif
> > > +       err = device_add(&pdev->dev);
> > > +       if (err) {
> > > +               kfree(pdev);
> > > +               return NULL;
> > > +       }
> > > +
> > > +       if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL)) {
> > > +               device_del(&pdev->dev);
> > > +               return NULL;
> > > +       }
> > > +
> > > +       i915 = devm_drm_dev_alloc(&pdev->dev, &mock_driver,
> > > +                                 struct drm_i915_private, drm);
> > > +       if (IS_ERR(i915)) {
> > > +               pr_err("Failed to allocate mock GEM device: err=%d\n", err);
> >
> > err = PTR_ERR(i915)
>
> Are you sure? We return a pointer here, and callers just expect NULL
> when stuff fails (so neither errno nor ptr-encoded errno).

I just meant for the pr_err() which is printing the err(from the
copy-paste), but it will always be zero without the above.

> -Daniel
>
> > Reviewed-by: Matthew Auld <matthew.auld at intel.com>
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch