diff mbox series

[v2] fbdev/efifb: Release PCI device's runtime PM ref during FB destroy

Message ID 20210809133146.2478382-1-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2] fbdev/efifb: Release PCI device's runtime PM ref during FB destroy | expand

Commit Message

Imre Deak Aug. 9, 2021, 1:31 p.m. UTC
Atm the EFI FB platform driver gets a runtime PM reference for the
associated GFX PCI device during probing the EFI FB platform device and
releases it only when the platform device gets unbound.

When fbcon switches to the FB provided by the PCI device's driver (for
instance i915/drmfb), the EFI FB will get only unregistered without the
EFI FB platform device getting unbound, keeping the runtime PM reference
acquired during the platform device probing. This reference will prevent
the PCI driver from runtime suspending the device.

Fix this by releasing the RPM reference from the EFI FB's destroy hook,
called when the FB gets unregistered.

While at it assert that pm_runtime_get_sync() didn't fail.

v2:
- Move pm_runtime_get_sync() before register_framebuffer() to avoid its
  race wrt. efifb_destroy()->pm_runtime_put(). (Daniel)
- Assert that pm_runtime_get_sync() didn't fail.
- Clarify commit message wrt. platform/PCI device/driver and driver
  removal vs. device unbinding.

Fixes: a6c0fd3d5a8b ("efifb: Ensure graphics device for efifb stays at PCI D0")
Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> (v1)
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/video/fbdev/efifb.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

Comments

Imre Deak Aug. 10, 2021, 8:36 a.m. UTC | #1
Hi Kai-Heng, Alex,

could you add your ack if the fix looks ok and you're ok if I push it to
the i915 tree?

Thanks,
Imre

On Mon, Aug 09, 2021 at 04:31:46PM +0300, Imre Deak wrote:
> Atm the EFI FB platform driver gets a runtime PM reference for the
> associated GFX PCI device during probing the EFI FB platform device and
> releases it only when the platform device gets unbound.
> 
> When fbcon switches to the FB provided by the PCI device's driver (for
> instance i915/drmfb), the EFI FB will get only unregistered without the
> EFI FB platform device getting unbound, keeping the runtime PM reference
> acquired during the platform device probing. This reference will prevent
> the PCI driver from runtime suspending the device.
> 
> Fix this by releasing the RPM reference from the EFI FB's destroy hook,
> called when the FB gets unregistered.
> 
> While at it assert that pm_runtime_get_sync() didn't fail.
> 
> v2:
> - Move pm_runtime_get_sync() before register_framebuffer() to avoid its
>   race wrt. efifb_destroy()->pm_runtime_put(). (Daniel)
> - Assert that pm_runtime_get_sync() didn't fail.
> - Clarify commit message wrt. platform/PCI device/driver and driver
>   removal vs. device unbinding.
> 
> Fixes: a6c0fd3d5a8b ("efifb: Ensure graphics device for efifb stays at PCI D0")
> Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> (v1)
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/video/fbdev/efifb.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> index 8ea8f079cde26..edca3703b9640 100644
> --- a/drivers/video/fbdev/efifb.c
> +++ b/drivers/video/fbdev/efifb.c
> @@ -47,6 +47,8 @@ static bool use_bgrt = true;
>  static bool request_mem_succeeded = false;
>  static u64 mem_flags = EFI_MEMORY_WC | EFI_MEMORY_UC;
>  
> +static struct pci_dev *efifb_pci_dev;	/* dev with BAR covering the efifb */
> +
>  static struct fb_var_screeninfo efifb_defined = {
>  	.activate		= FB_ACTIVATE_NOW,
>  	.height			= -1,
> @@ -243,6 +245,9 @@ static inline void efifb_show_boot_graphics(struct fb_info *info) {}
>  
>  static void efifb_destroy(struct fb_info *info)
>  {
> +	if (efifb_pci_dev)
> +		pm_runtime_put(&efifb_pci_dev->dev);
> +
>  	if (info->screen_base) {
>  		if (mem_flags & (EFI_MEMORY_UC | EFI_MEMORY_WC))
>  			iounmap(info->screen_base);
> @@ -333,7 +338,6 @@ ATTRIBUTE_GROUPS(efifb);
>  
>  static bool pci_dev_disabled;	/* FB base matches BAR of a disabled device */
>  
> -static struct pci_dev *efifb_pci_dev;	/* dev with BAR covering the efifb */
>  static struct resource *bar_resource;
>  static u64 bar_offset;
>  
> @@ -569,17 +573,22 @@ static int efifb_probe(struct platform_device *dev)
>  		pr_err("efifb: cannot allocate colormap\n");
>  		goto err_groups;
>  	}
> +
> +	if (efifb_pci_dev)
> +		WARN_ON(pm_runtime_get_sync(&efifb_pci_dev->dev) < 0);
> +
>  	err = register_framebuffer(info);
>  	if (err < 0) {
>  		pr_err("efifb: cannot register framebuffer\n");
> -		goto err_fb_dealoc;
> +		goto err_put_rpm_ref;
>  	}
>  	fb_info(info, "%s frame buffer device\n", info->fix.id);
> -	if (efifb_pci_dev)
> -		pm_runtime_get_sync(&efifb_pci_dev->dev);
>  	return 0;
>  
> -err_fb_dealoc:
> +err_put_rpm_ref:
> +	if (efifb_pci_dev)
> +		pm_runtime_put(&efifb_pci_dev->dev);
> +
>  	fb_dealloc_cmap(&info->cmap);
>  err_groups:
>  	sysfs_remove_groups(&dev->dev.kobj, efifb_groups);
> @@ -603,8 +612,6 @@ static int efifb_remove(struct platform_device *pdev)
>  	unregister_framebuffer(info);
>  	sysfs_remove_groups(&pdev->dev.kobj, efifb_groups);
>  	framebuffer_release(info);
> -	if (efifb_pci_dev)
> -		pm_runtime_put(&efifb_pci_dev->dev);
>  
>  	return 0;
>  }
> -- 
> 2.27.0
>
Alex Deucher Aug. 10, 2021, 2:48 p.m. UTC | #2
On Tue, Aug 10, 2021 at 4:36 AM Imre Deak <imre.deak@intel.com> wrote:
>
> Hi Kai-Heng, Alex,
>
> could you add your ack if the fix looks ok and you're ok if I push it to
> the i915 tree?
>

Acked-by: Alex Deucher <alexander.deucher@amd.com>

> Thanks,
> Imre
>
> On Mon, Aug 09, 2021 at 04:31:46PM +0300, Imre Deak wrote:
> > Atm the EFI FB platform driver gets a runtime PM reference for the
> > associated GFX PCI device during probing the EFI FB platform device and
> > releases it only when the platform device gets unbound.
> >
> > When fbcon switches to the FB provided by the PCI device's driver (for
> > instance i915/drmfb), the EFI FB will get only unregistered without the
> > EFI FB platform device getting unbound, keeping the runtime PM reference
> > acquired during the platform device probing. This reference will prevent
> > the PCI driver from runtime suspending the device.
> >
> > Fix this by releasing the RPM reference from the EFI FB's destroy hook,
> > called when the FB gets unregistered.
> >
> > While at it assert that pm_runtime_get_sync() didn't fail.
> >
> > v2:
> > - Move pm_runtime_get_sync() before register_framebuffer() to avoid its
> >   race wrt. efifb_destroy()->pm_runtime_put(). (Daniel)
> > - Assert that pm_runtime_get_sync() didn't fail.
> > - Clarify commit message wrt. platform/PCI device/driver and driver
> >   removal vs. device unbinding.
> >
> > Fixes: a6c0fd3d5a8b ("efifb: Ensure graphics device for efifb stays at PCI D0")
> > Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> (v1)
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/video/fbdev/efifb.c | 21 ++++++++++++++-------
> >  1 file changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> > index 8ea8f079cde26..edca3703b9640 100644
> > --- a/drivers/video/fbdev/efifb.c
> > +++ b/drivers/video/fbdev/efifb.c
> > @@ -47,6 +47,8 @@ static bool use_bgrt = true;
> >  static bool request_mem_succeeded = false;
> >  static u64 mem_flags = EFI_MEMORY_WC | EFI_MEMORY_UC;
> >
> > +static struct pci_dev *efifb_pci_dev;        /* dev with BAR covering the efifb */
> > +
> >  static struct fb_var_screeninfo efifb_defined = {
> >       .activate               = FB_ACTIVATE_NOW,
> >       .height                 = -1,
> > @@ -243,6 +245,9 @@ static inline void efifb_show_boot_graphics(struct fb_info *info) {}
> >
> >  static void efifb_destroy(struct fb_info *info)
> >  {
> > +     if (efifb_pci_dev)
> > +             pm_runtime_put(&efifb_pci_dev->dev);
> > +
> >       if (info->screen_base) {
> >               if (mem_flags & (EFI_MEMORY_UC | EFI_MEMORY_WC))
> >                       iounmap(info->screen_base);
> > @@ -333,7 +338,6 @@ ATTRIBUTE_GROUPS(efifb);
> >
> >  static bool pci_dev_disabled;        /* FB base matches BAR of a disabled device */
> >
> > -static struct pci_dev *efifb_pci_dev;        /* dev with BAR covering the efifb */
> >  static struct resource *bar_resource;
> >  static u64 bar_offset;
> >
> > @@ -569,17 +573,22 @@ static int efifb_probe(struct platform_device *dev)
> >               pr_err("efifb: cannot allocate colormap\n");
> >               goto err_groups;
> >       }
> > +
> > +     if (efifb_pci_dev)
> > +             WARN_ON(pm_runtime_get_sync(&efifb_pci_dev->dev) < 0);
> > +
> >       err = register_framebuffer(info);
> >       if (err < 0) {
> >               pr_err("efifb: cannot register framebuffer\n");
> > -             goto err_fb_dealoc;
> > +             goto err_put_rpm_ref;
> >       }
> >       fb_info(info, "%s frame buffer device\n", info->fix.id);
> > -     if (efifb_pci_dev)
> > -             pm_runtime_get_sync(&efifb_pci_dev->dev);
> >       return 0;
> >
> > -err_fb_dealoc:
> > +err_put_rpm_ref:
> > +     if (efifb_pci_dev)
> > +             pm_runtime_put(&efifb_pci_dev->dev);
> > +
> >       fb_dealloc_cmap(&info->cmap);
> >  err_groups:
> >       sysfs_remove_groups(&dev->dev.kobj, efifb_groups);
> > @@ -603,8 +612,6 @@ static int efifb_remove(struct platform_device *pdev)
> >       unregister_framebuffer(info);
> >       sysfs_remove_groups(&pdev->dev.kobj, efifb_groups);
> >       framebuffer_release(info);
> > -     if (efifb_pci_dev)
> > -             pm_runtime_put(&efifb_pci_dev->dev);
> >
> >       return 0;
> >  }
> > --
> > 2.27.0
> >
Kai-Heng Feng Aug. 10, 2021, 3:20 p.m. UTC | #3
On Tue, Aug 10, 2021 at 10:49 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Tue, Aug 10, 2021 at 4:36 AM Imre Deak <imre.deak@intel.com> wrote:
> >
> > Hi Kai-Heng, Alex,
> >
> > could you add your ack if the fix looks ok and you're ok if I push it to
> > the i915 tree?
> >
>
> Acked-by: Alex Deucher <alexander.deucher@amd.com>

Acked-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

>
> > Thanks,
> > Imre
> >
> > On Mon, Aug 09, 2021 at 04:31:46PM +0300, Imre Deak wrote:
> > > Atm the EFI FB platform driver gets a runtime PM reference for the
> > > associated GFX PCI device during probing the EFI FB platform device and
> > > releases it only when the platform device gets unbound.
> > >
> > > When fbcon switches to the FB provided by the PCI device's driver (for
> > > instance i915/drmfb), the EFI FB will get only unregistered without the
> > > EFI FB platform device getting unbound, keeping the runtime PM reference
> > > acquired during the platform device probing. This reference will prevent
> > > the PCI driver from runtime suspending the device.
> > >
> > > Fix this by releasing the RPM reference from the EFI FB's destroy hook,
> > > called when the FB gets unregistered.
> > >
> > > While at it assert that pm_runtime_get_sync() didn't fail.
> > >
> > > v2:
> > > - Move pm_runtime_get_sync() before register_framebuffer() to avoid its
> > >   race wrt. efifb_destroy()->pm_runtime_put(). (Daniel)
> > > - Assert that pm_runtime_get_sync() didn't fail.
> > > - Clarify commit message wrt. platform/PCI device/driver and driver
> > >   removal vs. device unbinding.
> > >
> > > Fixes: a6c0fd3d5a8b ("efifb: Ensure graphics device for efifb stays at PCI D0")
> > > Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> (v1)
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/video/fbdev/efifb.c | 21 ++++++++++++++-------
> > >  1 file changed, 14 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> > > index 8ea8f079cde26..edca3703b9640 100644
> > > --- a/drivers/video/fbdev/efifb.c
> > > +++ b/drivers/video/fbdev/efifb.c
> > > @@ -47,6 +47,8 @@ static bool use_bgrt = true;
> > >  static bool request_mem_succeeded = false;
> > >  static u64 mem_flags = EFI_MEMORY_WC | EFI_MEMORY_UC;
> > >
> > > +static struct pci_dev *efifb_pci_dev;        /* dev with BAR covering the efifb */
> > > +
> > >  static struct fb_var_screeninfo efifb_defined = {
> > >       .activate               = FB_ACTIVATE_NOW,
> > >       .height                 = -1,
> > > @@ -243,6 +245,9 @@ static inline void efifb_show_boot_graphics(struct fb_info *info) {}
> > >
> > >  static void efifb_destroy(struct fb_info *info)
> > >  {
> > > +     if (efifb_pci_dev)
> > > +             pm_runtime_put(&efifb_pci_dev->dev);
> > > +
> > >       if (info->screen_base) {
> > >               if (mem_flags & (EFI_MEMORY_UC | EFI_MEMORY_WC))
> > >                       iounmap(info->screen_base);
> > > @@ -333,7 +338,6 @@ ATTRIBUTE_GROUPS(efifb);
> > >
> > >  static bool pci_dev_disabled;        /* FB base matches BAR of a disabled device */
> > >
> > > -static struct pci_dev *efifb_pci_dev;        /* dev with BAR covering the efifb */
> > >  static struct resource *bar_resource;
> > >  static u64 bar_offset;
> > >
> > > @@ -569,17 +573,22 @@ static int efifb_probe(struct platform_device *dev)
> > >               pr_err("efifb: cannot allocate colormap\n");
> > >               goto err_groups;
> > >       }
> > > +
> > > +     if (efifb_pci_dev)
> > > +             WARN_ON(pm_runtime_get_sync(&efifb_pci_dev->dev) < 0);
> > > +
> > >       err = register_framebuffer(info);
> > >       if (err < 0) {
> > >               pr_err("efifb: cannot register framebuffer\n");
> > > -             goto err_fb_dealoc;
> > > +             goto err_put_rpm_ref;
> > >       }
> > >       fb_info(info, "%s frame buffer device\n", info->fix.id);
> > > -     if (efifb_pci_dev)
> > > -             pm_runtime_get_sync(&efifb_pci_dev->dev);
> > >       return 0;
> > >
> > > -err_fb_dealoc:
> > > +err_put_rpm_ref:
> > > +     if (efifb_pci_dev)
> > > +             pm_runtime_put(&efifb_pci_dev->dev);
> > > +
> > >       fb_dealloc_cmap(&info->cmap);
> > >  err_groups:
> > >       sysfs_remove_groups(&dev->dev.kobj, efifb_groups);
> > > @@ -603,8 +612,6 @@ static int efifb_remove(struct platform_device *pdev)
> > >       unregister_framebuffer(info);
> > >       sysfs_remove_groups(&pdev->dev.kobj, efifb_groups);
> > >       framebuffer_release(info);
> > > -     if (efifb_pci_dev)
> > > -             pm_runtime_put(&efifb_pci_dev->dev);
> > >
> > >       return 0;
> > >  }
> > > --
> > > 2.27.0
> > >
Imre Deak Aug. 13, 2021, 1:46 p.m. UTC | #4
On Mon, Aug 09, 2021 at 04:31:46PM +0300, Imre Deak wrote:
> Atm the EFI FB platform driver gets a runtime PM reference for the
> associated GFX PCI device during probing the EFI FB platform device and
> releases it only when the platform device gets unbound.
> 
> When fbcon switches to the FB provided by the PCI device's driver (for
> instance i915/drmfb), the EFI FB will get only unregistered without the
> EFI FB platform device getting unbound, keeping the runtime PM reference
> acquired during the platform device probing. This reference will prevent
> the PCI driver from runtime suspending the device.
> 
> Fix this by releasing the RPM reference from the EFI FB's destroy hook,
> called when the FB gets unregistered.
> 
> While at it assert that pm_runtime_get_sync() didn't fail.
> 
> v2:
> - Move pm_runtime_get_sync() before register_framebuffer() to avoid its
>   race wrt. efifb_destroy()->pm_runtime_put(). (Daniel)
> - Assert that pm_runtime_get_sync() didn't fail.
> - Clarify commit message wrt. platform/PCI device/driver and driver
>   removal vs. device unbinding.
> 
> Fixes: a6c0fd3d5a8b ("efifb: Ensure graphics device for efifb stays at PCI D0")
> Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> (v1)
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Thanks for the reviews, pushed to drm-intel-next.

> ---
>  drivers/video/fbdev/efifb.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> index 8ea8f079cde26..edca3703b9640 100644
> --- a/drivers/video/fbdev/efifb.c
> +++ b/drivers/video/fbdev/efifb.c
> @@ -47,6 +47,8 @@ static bool use_bgrt = true;
>  static bool request_mem_succeeded = false;
>  static u64 mem_flags = EFI_MEMORY_WC | EFI_MEMORY_UC;
>  
> +static struct pci_dev *efifb_pci_dev;	/* dev with BAR covering the efifb */
> +
>  static struct fb_var_screeninfo efifb_defined = {
>  	.activate		= FB_ACTIVATE_NOW,
>  	.height			= -1,
> @@ -243,6 +245,9 @@ static inline void efifb_show_boot_graphics(struct fb_info *info) {}
>  
>  static void efifb_destroy(struct fb_info *info)
>  {
> +	if (efifb_pci_dev)
> +		pm_runtime_put(&efifb_pci_dev->dev);
> +
>  	if (info->screen_base) {
>  		if (mem_flags & (EFI_MEMORY_UC | EFI_MEMORY_WC))
>  			iounmap(info->screen_base);
> @@ -333,7 +338,6 @@ ATTRIBUTE_GROUPS(efifb);
>  
>  static bool pci_dev_disabled;	/* FB base matches BAR of a disabled device */
>  
> -static struct pci_dev *efifb_pci_dev;	/* dev with BAR covering the efifb */
>  static struct resource *bar_resource;
>  static u64 bar_offset;
>  
> @@ -569,17 +573,22 @@ static int efifb_probe(struct platform_device *dev)
>  		pr_err("efifb: cannot allocate colormap\n");
>  		goto err_groups;
>  	}
> +
> +	if (efifb_pci_dev)
> +		WARN_ON(pm_runtime_get_sync(&efifb_pci_dev->dev) < 0);
> +
>  	err = register_framebuffer(info);
>  	if (err < 0) {
>  		pr_err("efifb: cannot register framebuffer\n");
> -		goto err_fb_dealoc;
> +		goto err_put_rpm_ref;
>  	}
>  	fb_info(info, "%s frame buffer device\n", info->fix.id);
> -	if (efifb_pci_dev)
> -		pm_runtime_get_sync(&efifb_pci_dev->dev);
>  	return 0;
>  
> -err_fb_dealoc:
> +err_put_rpm_ref:
> +	if (efifb_pci_dev)
> +		pm_runtime_put(&efifb_pci_dev->dev);
> +
>  	fb_dealloc_cmap(&info->cmap);
>  err_groups:
>  	sysfs_remove_groups(&dev->dev.kobj, efifb_groups);
> @@ -603,8 +612,6 @@ static int efifb_remove(struct platform_device *pdev)
>  	unregister_framebuffer(info);
>  	sysfs_remove_groups(&pdev->dev.kobj, efifb_groups);
>  	framebuffer_release(info);
> -	if (efifb_pci_dev)
> -		pm_runtime_put(&efifb_pci_dev->dev);
>  
>  	return 0;
>  }
> -- 
> 2.27.0
>
diff mbox series

Patch

diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 8ea8f079cde26..edca3703b9640 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -47,6 +47,8 @@  static bool use_bgrt = true;
 static bool request_mem_succeeded = false;
 static u64 mem_flags = EFI_MEMORY_WC | EFI_MEMORY_UC;
 
+static struct pci_dev *efifb_pci_dev;	/* dev with BAR covering the efifb */
+
 static struct fb_var_screeninfo efifb_defined = {
 	.activate		= FB_ACTIVATE_NOW,
 	.height			= -1,
@@ -243,6 +245,9 @@  static inline void efifb_show_boot_graphics(struct fb_info *info) {}
 
 static void efifb_destroy(struct fb_info *info)
 {
+	if (efifb_pci_dev)
+		pm_runtime_put(&efifb_pci_dev->dev);
+
 	if (info->screen_base) {
 		if (mem_flags & (EFI_MEMORY_UC | EFI_MEMORY_WC))
 			iounmap(info->screen_base);
@@ -333,7 +338,6 @@  ATTRIBUTE_GROUPS(efifb);
 
 static bool pci_dev_disabled;	/* FB base matches BAR of a disabled device */
 
-static struct pci_dev *efifb_pci_dev;	/* dev with BAR covering the efifb */
 static struct resource *bar_resource;
 static u64 bar_offset;
 
@@ -569,17 +573,22 @@  static int efifb_probe(struct platform_device *dev)
 		pr_err("efifb: cannot allocate colormap\n");
 		goto err_groups;
 	}
+
+	if (efifb_pci_dev)
+		WARN_ON(pm_runtime_get_sync(&efifb_pci_dev->dev) < 0);
+
 	err = register_framebuffer(info);
 	if (err < 0) {
 		pr_err("efifb: cannot register framebuffer\n");
-		goto err_fb_dealoc;
+		goto err_put_rpm_ref;
 	}
 	fb_info(info, "%s frame buffer device\n", info->fix.id);
-	if (efifb_pci_dev)
-		pm_runtime_get_sync(&efifb_pci_dev->dev);
 	return 0;
 
-err_fb_dealoc:
+err_put_rpm_ref:
+	if (efifb_pci_dev)
+		pm_runtime_put(&efifb_pci_dev->dev);
+
 	fb_dealloc_cmap(&info->cmap);
 err_groups:
 	sysfs_remove_groups(&dev->dev.kobj, efifb_groups);
@@ -603,8 +612,6 @@  static int efifb_remove(struct platform_device *pdev)
 	unregister_framebuffer(info);
 	sysfs_remove_groups(&pdev->dev.kobj, efifb_groups);
 	framebuffer_release(info);
-	if (efifb_pci_dev)
-		pm_runtime_put(&efifb_pci_dev->dev);
 
 	return 0;
 }