From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: Matthew Auld <matthew.william.auld@gmail.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>,
Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
amd-gfx list <amd-gfx@lists.freedesktop.org>,
DRI Development <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 2/4] drm/i915/selftests: align more to real device lifetimes
Date: Fri, 18 Sep 2020 20:46:20 +0200 [thread overview]
Message-ID: <CAKMK7uH+CnVrzrGoMAq2dFnTQWigmb36qTJ8p4fJx9vAwig3Fw@mail.gmail.com> (raw)
In-Reply-To: <CAM0jSHMD5LxEHqbdP8wfRr_6SUahjddGRLVdcF2Q47GUxOhk2A@mail.gmail.com>
On Fri, Sep 18, 2020 at 8:31 PM Matthew Auld
<matthew.william.auld@gmail.com> wrote:
>
> On Fri, 18 Sep 2020 at 19:22, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >
> > On Fri, Sep 18, 2020 at 7:50 PM Matthew Auld
> > <matthew.william.auld@gmail.com> wrote:
> > >
> > > On Fri, 18 Sep 2020 at 14:25, Daniel Vetter <daniel.vetter@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@gmail.com>
> > > > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (v3)
> > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@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.
Ah right, I missed that when applying your previous comment, thanks
for pointing it out.
-Daniel
>
> > -Daniel
> >
> > > Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> >
> >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
next prev parent reply other threads:[~2020-09-18 18:46 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-18 13:25 [PATCH 0/4] managed drm_device, absolute final leftover bits Daniel Vetter
2020-09-18 13:25 ` [PATCH 1/4] drm/i915/selftest: Create mock_destroy_device Daniel Vetter
2020-09-18 13:25 ` [PATCH 2/4] drm/i915/selftests: align more to real device lifetimes Daniel Vetter
2020-09-18 17:50 ` Matthew Auld
2020-09-18 18:22 ` Daniel Vetter
2020-09-18 18:31 ` Matthew Auld
2020-09-18 18:46 ` Daniel Vetter [this message]
2020-09-18 20:00 ` [PATCH] " Daniel Vetter
2020-09-19 13:38 ` Daniel Vetter
2020-09-19 13:40 ` Daniel Vetter
2020-09-18 13:25 ` [PATCH 3/4] drm/amdgpu: Convert to using devm_drm_dev_alloc() (v2) Daniel Vetter
2020-09-18 13:25 ` [PATCH 4/4] drm/dev: Remove drm_dev_init Daniel Vetter
2020-09-18 14:26 ` Thomas Zimmermann
2020-09-18 15:03 ` [PATCH 0/4] managed drm_device, absolute final leftover bits Alex Deucher
2020-09-18 17:12 ` [Intel-gfx] " Rodrigo Vivi
2020-09-21 9:09 ` Daniel Vetter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKMK7uH+CnVrzrGoMAq2dFnTQWigmb36qTJ8p4fJx9vAwig3Fw@mail.gmail.com \
--to=daniel.vetter@ffwll.ch \
--cc=amd-gfx@lists.freedesktop.org \
--cc=daniel.vetter@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=matthew.william.auld@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).