Re: [RFC v3 1/3] fbmem: add default get_fb_unmapped_area function

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Thursday 01 Dec 2016 08:27:58 Benjamin Gaignard wrote:
> 2016-12-01 3:31 GMT+01:00 Michel Dänzer <michel@daenzer.net>:
> > On 01/12/16 04:46 AM, Benjamin Gaignard wrote:
> >> 2016-11-30 20:39 GMT+01:00 Laurent Pinchart:
> >>> On Wednesday 30 Nov 2016 20:34:17 Benjamin Gaignard wrote:
> >>>> If HAVE_ARCH_FB_UNMAPPED_AREA is set and get_fb_unmapped_area
> >>>> function not defined in platform architecture let use a default
> >>>> function.
> >>>> 
> >>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> >>>> ---
> >>>> 
> >>>>  drivers/video/fbdev/core/fbmem.c | 15 +++++++++++++++
> >>>>  1 file changed, 15 insertions(+)
> >>>> 
> >>>> diff --git a/drivers/video/fbdev/core/fbmem.c
> >>>> b/drivers/video/fbdev/core/fbmem.c index 76c1ad9..54488ee 100644
> >>>> --- a/drivers/video/fbdev/core/fbmem.c
> >>>> +++ b/drivers/video/fbdev/core/fbmem.c
> >>>> @@ -1492,6 +1492,21 @@ static long fb_compat_ioctl(struct file *file,
> >>>> unsigned int cmd, return 0;
> >>>> 
> >>>>  }
> >>>> 
> >>>> +#if defined(HAVE_ARCH_FB_UNMAPPED_AREA) &&
> >>>> !defined(get_fb_unmapped_area)
> >>> 
> >>> I think I've asked this twice already, how is that possible ?
> >> 
> >> As you said before only sparc and blackfin architectures have define
> >> HAVE_ARCH_FB_UNMAPPED_AREA
> >> and get_fb_unmapped_area().
> >> For stm32f4 (ARMv7m) I plan to only add HAVE_ARCH_FB_UNMAPPED_AREA and
> >> use this default function
> > 
> > Note that the preprocessor only considers defined(get_fb_unmapped_area)
> > true if get_fb_unmapped_area is defined as a macro, not if it's a normal
> > function. Since AFAICT get_fb_unmapped_area is a normal function on
> > sparc and blackfin, this default function will be compiled on those
> > architectures as well.
> 
> Since only sparc and blackfin have this function already, I will
> change the condition to:
> #ifdef HAVE_ARCH_FB_UNMAPPED_AREA
> #ifndef CONFIG_SPARC
> #ifndef CONFIG_BLACKFIN

No, that's really hackish. HAVE_ARCH_FB_UNMAPPED_AREA means that the 
architecture provides the function. The generic version should not be provided 
when that macro is defined.

Please try to see if you can't share the implementation with at least 
blackfin, and possibly sparc. If sparc requires its own version, you need a 
different mechanism to decide whether to provide the generic one.

> so this function won't be compiled for those architectures

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel