All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Export intel_framebuffer_fini
@ 2013-08-06 16:43 Chris Wilson
  2013-08-06 17:13 ` Ville Syrjälä
  0 siblings, 1 reply; 3+ messages in thread
From: Chris Wilson @ 2013-08-06 16:43 UTC (permalink / raw)
  To: intel-gfx

Rather than open-code the teardown of a framebuffer, export the routine
from intel_display.c. This then make intel_fbdev symmetric in its use of
the common intel_framebuffer routines to initialise and clean up the
struct intel_framebuffer. (And new features need only be added in one
location!)

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c | 10 +++++++---
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_fb.c      | 15 +++++----------
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0584480..dc52fea 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9438,13 +9438,17 @@ static void intel_setup_outputs(struct drm_device *dev)
 	drm_helper_move_panel_connectors_to_head(dev);
 }
 
+void intel_framebuffer_fini(struct intel_framebuffer *fb)
+{
+	drm_framebuffer_cleanup(&fb->base);
+	drm_gem_object_unreference_unlocked(&fb->obj->base);
+}
+
 static void intel_user_framebuffer_destroy(struct drm_framebuffer *fb)
 {
 	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
 
-	drm_framebuffer_cleanup(fb);
-	drm_gem_object_unreference_unlocked(&intel_fb->obj->base);
-
+	intel_framebuffer_fini(intel_fb);
 	kfree(intel_fb);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ed33976..54e389d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -720,6 +720,7 @@ extern int intel_framebuffer_init(struct drm_device *dev,
 				  struct intel_framebuffer *ifb,
 				  struct drm_mode_fb_cmd2 *mode_cmd,
 				  struct drm_i915_gem_object *obj);
+extern void intel_framebuffer_fini(struct intel_framebuffer *fb);
 extern int intel_fbdev_init(struct drm_device *dev);
 extern void intel_fbdev_initial_config(struct drm_device *dev);
 extern void intel_fbdev_fini(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index f3c97e0..bc21000 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -193,26 +193,21 @@ static struct drm_fb_helper_funcs intel_fb_helper_funcs = {
 static void intel_fbdev_destroy(struct drm_device *dev,
 				struct intel_fbdev *ifbdev)
 {
-	struct fb_info *info;
-	struct intel_framebuffer *ifb = &ifbdev->ifb;
-
 	if (ifbdev->helper.fbdev) {
-		info = ifbdev->helper.fbdev;
+		struct fb_info *info = ifbdev->helper.fbdev;
+
 		unregister_framebuffer(info);
 		iounmap(info->screen_base);
 		if (info->cmap.len)
 			fb_dealloc_cmap(&info->cmap);
+
 		framebuffer_release(info);
 	}
 
 	drm_fb_helper_fini(&ifbdev->helper);
 
-	drm_framebuffer_unregister_private(&ifb->base);
-	drm_framebuffer_cleanup(&ifb->base);
-	if (ifb->obj) {
-		drm_gem_object_unreference_unlocked(&ifb->obj->base);
-		ifb->obj = NULL;
-	}
+	drm_framebuffer_unregister_private(&ifbdev->ifb.base);
+	intel_framebuffer_fini(&ifbdev->ifb);
 }
 
 int intel_fbdev_init(struct drm_device *dev)
-- 
1.8.4.rc1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/i915: Export intel_framebuffer_fini
  2013-08-06 16:43 [PATCH] drm/i915: Export intel_framebuffer_fini Chris Wilson
@ 2013-08-06 17:13 ` Ville Syrjälä
  2013-08-06 18:09   ` Daniel Vetter
  0 siblings, 1 reply; 3+ messages in thread
From: Ville Syrjälä @ 2013-08-06 17:13 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Tue, Aug 06, 2013 at 05:43:07PM +0100, Chris Wilson wrote:
> Rather than open-code the teardown of a framebuffer, export the routine
> from intel_display.c. This then make intel_fbdev symmetric in its use of
> the common intel_framebuffer routines to initialise and clean up the
> struct intel_framebuffer. (And new features need only be added in one
> location!)
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 10 +++++++---
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_fb.c      | 15 +++++----------
>  3 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0584480..dc52fea 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9438,13 +9438,17 @@ static void intel_setup_outputs(struct drm_device *dev)
>  	drm_helper_move_panel_connectors_to_head(dev);
>  }
>  
> +void intel_framebuffer_fini(struct intel_framebuffer *fb)
> +{
> +	drm_framebuffer_cleanup(&fb->base);
> +	drm_gem_object_unreference_unlocked(&fb->obj->base);
> +}
> +
>  static void intel_user_framebuffer_destroy(struct drm_framebuffer *fb)
>  {
>  	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
>  
> -	drm_framebuffer_cleanup(fb);
> -	drm_gem_object_unreference_unlocked(&intel_fb->obj->base);
> -
> +	intel_framebuffer_fini(intel_fb);
>  	kfree(intel_fb);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index ed33976..54e389d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -720,6 +720,7 @@ extern int intel_framebuffer_init(struct drm_device *dev,
>  				  struct intel_framebuffer *ifb,
>  				  struct drm_mode_fb_cmd2 *mode_cmd,
>  				  struct drm_i915_gem_object *obj);
> +extern void intel_framebuffer_fini(struct intel_framebuffer *fb);
>  extern int intel_fbdev_init(struct drm_device *dev);
>  extern void intel_fbdev_initial_config(struct drm_device *dev);
>  extern void intel_fbdev_fini(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> index f3c97e0..bc21000 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -193,26 +193,21 @@ static struct drm_fb_helper_funcs intel_fb_helper_funcs = {
>  static void intel_fbdev_destroy(struct drm_device *dev,
>  				struct intel_fbdev *ifbdev)
>  {
> -	struct fb_info *info;
> -	struct intel_framebuffer *ifb = &ifbdev->ifb;
> -
>  	if (ifbdev->helper.fbdev) {
> -		info = ifbdev->helper.fbdev;
> +		struct fb_info *info = ifbdev->helper.fbdev;
> +
>  		unregister_framebuffer(info);
>  		iounmap(info->screen_base);
>  		if (info->cmap.len)
>  			fb_dealloc_cmap(&info->cmap);
> +
>  		framebuffer_release(info);
>  	}
>  
>  	drm_fb_helper_fini(&ifbdev->helper);
>  
> -	drm_framebuffer_unregister_private(&ifb->base);
> -	drm_framebuffer_cleanup(&ifb->base);
> -	if (ifb->obj) {
> -		drm_gem_object_unreference_unlocked(&ifb->obj->base);
> -		ifb->obj = NULL;
> -	}

I wonder what this thing was trying to achieve. Almost like it was
expecting to be called before .fb_probe() happened, but that doesn't
appear to be possible. And if it did happen, the fb itself wouldn't
have been initialized either so it would explode anyway.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +	drm_framebuffer_unregister_private(&ifbdev->ifb.base);
> +	intel_framebuffer_fini(&ifbdev->ifb);
>  }
>  
>  int intel_fbdev_init(struct drm_device *dev)
> -- 
> 1.8.4.rc1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/i915: Export intel_framebuffer_fini
  2013-08-06 17:13 ` Ville Syrjälä
@ 2013-08-06 18:09   ` Daniel Vetter
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Vetter @ 2013-08-06 18:09 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Tue, Aug 06, 2013 at 08:13:04PM +0300, Ville Syrjälä wrote:
> On Tue, Aug 06, 2013 at 05:43:07PM +0100, Chris Wilson wrote:
> > Rather than open-code the teardown of a framebuffer, export the routine
> > from intel_display.c. This then make intel_fbdev symmetric in its use of
> > the common intel_framebuffer routines to initialise and clean up the
> > struct intel_framebuffer. (And new features need only be added in one
> > location!)
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 10 +++++++---
> >  drivers/gpu/drm/i915/intel_drv.h     |  1 +
> >  drivers/gpu/drm/i915/intel_fb.c      | 15 +++++----------
> >  3 files changed, 13 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 0584480..dc52fea 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9438,13 +9438,17 @@ static void intel_setup_outputs(struct drm_device *dev)
> >  	drm_helper_move_panel_connectors_to_head(dev);
> >  }
> >  
> > +void intel_framebuffer_fini(struct intel_framebuffer *fb)
> > +{
> > +	drm_framebuffer_cleanup(&fb->base);
> > +	drm_gem_object_unreference_unlocked(&fb->obj->base);
> > +}
> > +
> >  static void intel_user_framebuffer_destroy(struct drm_framebuffer *fb)
> >  {
> >  	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
> >  
> > -	drm_framebuffer_cleanup(fb);
> > -	drm_gem_object_unreference_unlocked(&intel_fb->obj->base);
> > -
> > +	intel_framebuffer_fini(intel_fb);
> >  	kfree(intel_fb);
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index ed33976..54e389d 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -720,6 +720,7 @@ extern int intel_framebuffer_init(struct drm_device *dev,
> >  				  struct intel_framebuffer *ifb,
> >  				  struct drm_mode_fb_cmd2 *mode_cmd,
> >  				  struct drm_i915_gem_object *obj);
> > +extern void intel_framebuffer_fini(struct intel_framebuffer *fb);
> >  extern int intel_fbdev_init(struct drm_device *dev);
> >  extern void intel_fbdev_initial_config(struct drm_device *dev);
> >  extern void intel_fbdev_fini(struct drm_device *dev);
> > diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> > index f3c97e0..bc21000 100644
> > --- a/drivers/gpu/drm/i915/intel_fb.c
> > +++ b/drivers/gpu/drm/i915/intel_fb.c
> > @@ -193,26 +193,21 @@ static struct drm_fb_helper_funcs intel_fb_helper_funcs = {
> >  static void intel_fbdev_destroy(struct drm_device *dev,
> >  				struct intel_fbdev *ifbdev)
> >  {
> > -	struct fb_info *info;
> > -	struct intel_framebuffer *ifb = &ifbdev->ifb;
> > -
> >  	if (ifbdev->helper.fbdev) {
> > -		info = ifbdev->helper.fbdev;
> > +		struct fb_info *info = ifbdev->helper.fbdev;
> > +
> >  		unregister_framebuffer(info);
> >  		iounmap(info->screen_base);
> >  		if (info->cmap.len)
> >  			fb_dealloc_cmap(&info->cmap);
> > +
> >  		framebuffer_release(info);
> >  	}
> >  
> >  	drm_fb_helper_fini(&ifbdev->helper);
> >  
> > -	drm_framebuffer_unregister_private(&ifb->base);
> > -	drm_framebuffer_cleanup(&ifb->base);
> > -	if (ifb->obj) {
> > -		drm_gem_object_unreference_unlocked(&ifb->obj->base);
> > -		ifb->obj = NULL;
> > -	}
> 
> I wonder what this thing was trying to achieve. Almost like it was
> expecting to be called before .fb_probe() happened, but that doesn't
> appear to be possible. And if it did happen, the fb itself wouldn't
> have been initialized either so it would explode anyway.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-08-06 18:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-06 16:43 [PATCH] drm/i915: Export intel_framebuffer_fini Chris Wilson
2013-08-06 17:13 ` Ville Syrjälä
2013-08-06 18:09   ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.