[v2,02/12] fbdev: allow apertures == NULL in remove_conflicting_framebuffers()

Interpret (otherwise-invalid) NULL apertures argument to mean all-memory
range. This will allow to remove several duplicates of this code from
drivers in following patches.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
[for v1]
Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

---
v2: added kerneldoc to corresponding DRM helper
---
 drivers/video/fbdev/core/fbmem.c | 14 ++++++++++++++
 include/drm/drm_fb_helper.h      | 10 ++++++++++
 2 files changed, 24 insertions(+)

On Thu, Aug 30, 2018 at 11:00:05PM +0200, Michał Mirosław wrote:
> Interpret (otherwise-invalid) NULL apertures argument to mean all-memory
> range. This will allow to remove several duplicates of this code from
> drivers in following patches.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> [for v1]
> Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> 
> ---
> v2: added kerneldoc to corresponding DRM helper
> ---
>  drivers/video/fbdev/core/fbmem.c | 14 ++++++++++++++
>  include/drm/drm_fb_helper.h      | 10 ++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index 30a18d4c9de4..0df148eb4699 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1779,11 +1779,25 @@ int remove_conflicting_framebuffers(struct apertures_struct *a,
>  				    const char *name, bool primary)
>  {
>  	int ret;
> +	bool do_free = false;
> +
> +	if (!a) {
> +		a = alloc_apertures(1);
> +		if (!a)
> +			return -ENOMEM;
> +
> +		a->ranges[0].base = 0;
> +		a->ranges[0].size = ~0;
> +		do_free = true;
> +	}
>  
>  	mutex_lock(&registration_lock);
>  	ret = do_remove_conflicting_framebuffers(a, name, primary);
>  	mutex_unlock(&registration_lock);
>  
> +	if (do_free)
> +		kfree(a);
> +
>  	return ret;
>  }
>  EXPORT_SYMBOL(remove_conflicting_framebuffers);
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index b069433e7fc1..1c1e53abb25d 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -566,6 +566,16 @@ static inline void drm_fb_helper_output_poll_changed(struct drm_device *dev)
>  
>  #endif
>  
> +/**
> + * drm_fb_helper_remove_conflicting_framebuffers - remove firmware framebuffers
> + * @a: memory range, users of which are to be removed
> + * @name: requesting driver name
> + * @primary: also kick vga16fb if present
> + *
> + * This function removes framebuffer devices (eg. initialized by firmware)
> + * which use memory range described by @a. If @a is NULL all such devices are
> + * removed.
> + */

This looks like misplaced copypasta. You only need this once I think.
-Daniel

>  static inline int
>  drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a,
>  					      const char *name, bool primary)
> -- 
> 2.18.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

On Fri, Aug 31, 2018 at 10:56:56AM +0200, Daniel Vetter wrote:
> On Thu, Aug 30, 2018 at 11:00:05PM +0200, Michał Mirosław wrote:
> > Interpret (otherwise-invalid) NULL apertures argument to mean all-memory
> > range. This will allow to remove several duplicates of this code from
> > drivers in following patches.
> > 
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > [for v1]
> > Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > 
> > ---
> > v2: added kerneldoc to corresponding DRM helper
> > ---
> >  drivers/video/fbdev/core/fbmem.c | 14 ++++++++++++++
> >  include/drm/drm_fb_helper.h      | 10 ++++++++++
> >  2 files changed, 24 insertions(+)
> > 
> > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> > index 30a18d4c9de4..0df148eb4699 100644
> > --- a/drivers/video/fbdev/core/fbmem.c
> > +++ b/drivers/video/fbdev/core/fbmem.c
> > @@ -1779,11 +1779,25 @@ int remove_conflicting_framebuffers(struct apertures_struct *a,
> >  				    const char *name, bool primary)
> >  {
> >  	int ret;
> > +	bool do_free = false;
> > +
> > +	if (!a) {
> > +		a = alloc_apertures(1);
> > +		if (!a)
> > +			return -ENOMEM;
> > +
> > +		a->ranges[0].base = 0;
> > +		a->ranges[0].size = ~0;
> > +		do_free = true;
> > +	}
> >  
> >  	mutex_lock(&registration_lock);
> >  	ret = do_remove_conflicting_framebuffers(a, name, primary);
> >  	mutex_unlock(&registration_lock);
> >  
> > +	if (do_free)
> > +		kfree(a);
> > +
> >  	return ret;
> >  }
> >  EXPORT_SYMBOL(remove_conflicting_framebuffers);
> > diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> > index b069433e7fc1..1c1e53abb25d 100644
> > --- a/include/drm/drm_fb_helper.h
> > +++ b/include/drm/drm_fb_helper.h
> > @@ -566,6 +566,16 @@ static inline void drm_fb_helper_output_poll_changed(struct drm_device *dev)
> >  
> >  #endif
> >  
> > +/**
> > + * drm_fb_helper_remove_conflicting_framebuffers - remove firmware framebuffers
> > + * @a: memory range, users of which are to be removed
> > + * @name: requesting driver name
> > + * @primary: also kick vga16fb if present
> > + *
> > + * This function removes framebuffer devices (eg. initialized by firmware)
> > + * which use memory range described by @a. If @a is NULL all such devices are
> > + * removed.
> > + */
> 
> This looks like misplaced copypasta. You only need this once I think.

Ah no, just a fixup for the lack of kerneldoc we have. Can you pls split
this out into a separate patch?

Thanks, Daniel

> -Daniel
> 
> >  static inline int
> >  drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a,
> >  					      const char *name, bool primary)
> > -- 
> > 2.18.0
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch