Re: [Outreachy kernel] Re: [PATCH v4] drm/i915: Replace *_reference/unreference() or *_ref/unref with _get/put()

From: Daniel Vetter
Date: Fri Oct 13 2017 - 10:58:05 EST


On Fri, Oct 13, 2017 at 04:51:38PM +0200, Daniel Vetter wrote:
> On Fri, Oct 13, 2017 at 08:03:59PM +0530, Harsha Sharma wrote:
> > On Mon, Oct 9, 2017 at 5:36 PM, Harsha Sharma
> > <harshasharmaiitr@xxxxxxxxx> wrote:
> > > Replace instances of drm_framebuffer_reference/unreference() with
> > > *_get/put() suffixes and drm_dev_unref with *_put() suffix
> > > because get/put is shorter and consistent with the
> > > kernel use of *_get/put suffixes.
> > > Done with following coccinelle semantic patch
> > >
> > > @@
> > > expression ex;
> > > @@
> > >
> > > (
> > > -drm_framebuffer_unreference(ex);
> > > +drm_framebuffer_put(ex);
> > > |
> > > -drm_dev_unref(ex);
> > > +drm_dev_put(ex);
> > > |
> > > -drm_framebuffer_reference(ex);
> > > +drm_framebuffer_get(ex);
> > > )
> > >
> > > Signed-off-by: Harsha Sharma <harshasharmaiitr@xxxxxxxxx>
> > > ---
> > > Changes in v4:
> > > -change one instance of *_put to *_get
> > > Changes in v3:
> > > -Removed changes in selftests
> > > Changes in v2:
> > > -Added cocinelle patch in log message
> > > -cc to all driver-specific mailing lists
> > >
> > > drivers/gpu/drm/i915/i915_pci.c | 2 +-
> > > drivers/gpu/drm/i915/intel_display.c | 10 +++++-----
> > > drivers/gpu/drm/i915/intel_fbdev.c | 4 ++--
> > > 3 files changed, 8 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> > > index 09d97e0990b7..2f106cca46b4 100644
> > > --- a/drivers/gpu/drm/i915/i915_pci.c
> > > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > > @@ -510,7 +510,7 @@ static void i915_pci_remove(struct pci_dev *pdev)
> > > struct drm_device *dev = pci_get_drvdata(pdev);
> > >
> > > i915_driver_unload(dev);
> > > - drm_dev_unref(dev);
> > > + drm_dev_put(dev);
> > > }
> > >
> > > static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 64f7b51ed97c..cb62f7e5d59a 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -2856,7 +2856,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
> > >
> > > if (intel_plane_ggtt_offset(state) == plane_config->base) {
> > > fb = c->primary->fb;
> > > - drm_framebuffer_reference(fb);
> > > + drm_framebuffer_get(fb);
> > > goto valid_fb;
> > > }
> > > }
> > > @@ -2887,7 +2887,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
> > > intel_crtc->pipe, PTR_ERR(intel_state->vma));
> > >
> > > intel_state->vma = NULL;
> > > - drm_framebuffer_unreference(fb);
> > > + drm_framebuffer_put(fb);
> > > return;
> > > }
> > >
> > > @@ -2908,7 +2908,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
> > > if (i915_gem_object_is_tiled(obj))
> > > dev_priv->preserve_bios_swizzle = true;
> > >
> > > - drm_framebuffer_reference(fb);
> > > + drm_framebuffer_get(fb);
> > > primary->fb = primary->state->fb = fb;
> > > primary->crtc = primary->state->crtc = &intel_crtc->base;
> > >
> > > @@ -9847,7 +9847,7 @@ mode_fits_in_fbdev(struct drm_device *dev,
> > > if (obj->base.size < mode->vdisplay * fb->pitches[0])
> > > return NULL;
> > >
> > > - drm_framebuffer_reference(fb);
> > > + drm_framebuffer_get(fb);
> > > return fb;
> > > #else
> > > return NULL;
> > > @@ -10028,7 +10028,7 @@ int intel_get_load_detect_pipe(struct drm_connector *connector,
> > > if (ret)
> > > goto fail;
> > >
> > > - drm_framebuffer_unreference(fb);
> > > + drm_framebuffer_put(fb);
> > >
> > > ret = drm_atomic_set_mode_for_crtc(&crtc_state->base, mode);
> > > if (ret)
> > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> > > index 262e75c00dd2..e34334a1fbf9 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > > @@ -189,7 +189,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
> > > " releasing it\n",
> > > intel_fb->base.width, intel_fb->base.height,
> > > sizes->fb_width, sizes->fb_height);
> > > - drm_framebuffer_unreference(&intel_fb->base);
> > > + drm_framebuffer_put(&intel_fb->base);
> > > intel_fb = ifbdev->fb = NULL;
> > > }
> > > if (!intel_fb || WARN_ON(!intel_fb->obj)) {
> > > @@ -624,7 +624,7 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
> > > ifbdev->preferred_bpp = fb->base.format->cpp[0] * 8;
> > > ifbdev->fb = fb;
> > >
> > > - drm_framebuffer_reference(&ifbdev->fb->base);
> > > + drm_framebuffer_get(&ifbdev->fb->base);
> > >
> > > /* Final pass to check if any active pipes don't have fbs */
> > > for_each_crtc(dev, crtc) {
> > >
> > I hope that this one can be merged.
> > Please let me know if any change is required.
> > Thanks for your time.
>
> Sorry, fell through the cracks.
>
> Applied now to drm-intel-next-queued, thanks for your patch.

Ok just realized that it doesn't apply on plain drm-intel-next-queued
since that tree doesn't yet have the drm_dev_put function. I've taken that
part out of your patch to unblock it.

Can you pls rebase the remaining parts onto drm-tip from

https://cgit.freedesktop.org/drm/drm-tip

and send a new patch for just the drm_dev_put change? I asked the
drm-intel maintainers to do the backmerge to make sure I can apply this
next week.

Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch