| Message ID | 1480534459-7239-2-git-send-email-benjamin.gaignard@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 ? > +unsigned long get_fb_unmapped_area(struct file *filp, > + unsigned long addr, unsigned long len, > + unsigned long pgoff, unsigned long flags) > +{ > + struct fb_info * const info = filp->private_data; > + unsigned long fb_size = PAGE_ALIGN(info->fix.smem_len); > + > + if (pgoff > fb_size || len > fb_size - pgoff) > + return -EINVAL; > + > + return (unsigned long)info->screen_base + pgoff; > +} > +#endif > + > static const struct file_operations fb_fops = { > .owner = THIS_MODULE, > .read = fb_read,
2016-11-30 20:39 GMT+01:00 Laurent Pinchart <laurent.pinchart@ideasonboard.com>: > 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 By testing those two flags I avoid to define twince get_fb_unmapped_area() if the architecture already provide it. > > >> +unsigned long get_fb_unmapped_area(struct file *filp, >> + unsigned long addr, unsigned long len, >> + unsigned long pgoff, unsigned long flags) >> +{ >> + struct fb_info * const info = filp->private_data; >> + unsigned long fb_size = PAGE_ALIGN(info->fix.smem_len); >> + >> + if (pgoff > fb_size || len > fb_size - pgoff) >> + return -EINVAL; >> + >> + return (unsigned long)info->screen_base + pgoff; >> +} >> +#endif >> + >> static const struct file_operations fb_fops = { >> .owner = THIS_MODULE, >> .read = fb_read, > > -- > Regards, > > Laurent Pinchart >
Hi Benjamin, On Wednesday 30 Nov 2016 20:46:23 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 Ah, so it's for support of a future patch. You should explain that in the commit message. The other question I asked was whether it would be possible to come up with a generic implementation of this function that would work on all architectures. At first sight the blackfin implementation is similar, but the sparc one is quite different. Why does it have to be arch-specific ? > By testing those two flags I avoid to define twince > get_fb_unmapped_area() if the architecture already > provide it. > > >> +unsigned long get_fb_unmapped_area(struct file *filp, > >> + unsigned long addr, unsigned long len, > >> + unsigned long pgoff, unsigned long > >> flags) > >> +{ > >> + struct fb_info * const info = filp->private_data; > >> + unsigned long fb_size = PAGE_ALIGN(info->fix.smem_len); > >> + > >> + if (pgoff > fb_size || len > fb_size - pgoff) > >> + return -EINVAL; > >> + > >> + return (unsigned long)info->screen_base + pgoff; > >> +} > >> +#endif > >> + > >> static const struct file_operations fb_fops = { > >> .owner = THIS_MODULE, > >> .read = fb_read,
On 01/12/16 04:46 AM, Benjamin Gaignard wrote: > 2016-11-30 20:39 GMT+01:00 Laurent Pinchart <laurent.pinchart@ideasonboard.com>: >> 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.
2016-12-01 2:13 GMT+01:00 Laurent Pinchart <laurent.pinchart@ideasonboard.com>: > Hi Benjamin, > > On Wednesday 30 Nov 2016 20:46:23 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 > > Ah, so it's for support of a future patch. You should explain that in the > commit message. OK > The other question I asked was whether it would be possible to come up with a > generic implementation of this function that would work on all architectures. > At first sight the blackfin implementation is similar, but the sparc one is > quite different. Why does it have to be arch-specific ? I don't know but I guess it is link to how memory is mmapped by the architecture >> By testing those two flags I avoid to define twince >> get_fb_unmapped_area() if the architecture already >> provide it. >> >> >> +unsigned long get_fb_unmapped_area(struct file *filp, >> >> + unsigned long addr, unsigned long len, >> >> + unsigned long pgoff, unsigned long >> >> flags) >> >> +{ >> >> + struct fb_info * const info = filp->private_data; >> >> + unsigned long fb_size = PAGE_ALIGN(info->fix.smem_len); >> >> + >> >> + if (pgoff > fb_size || len > fb_size - pgoff) >> >> + return -EINVAL; >> >> + >> >> + return (unsigned long)info->screen_base + pgoff; >> >> +} >> >> +#endif >> >> + >> >> static const struct file_operations fb_fops = { >> >> .owner = THIS_MODULE, >> >> .read = fb_read, > > -- > Regards, > > Laurent Pinchart >
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 <laurent.pinchart@ideasonboard.com>: >>> 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 so this function won't be compiled for those architectures > > -- > Earthling Michel Dänzer | http://www.amd.com > Libre software enthusiast | Mesa and X developer
Hi Benjamin, On Thursday 01 Dec 2016 08:24:50 Benjamin Gaignard wrote: > 2016-12-01 2:13 GMT+01:00 Laurent Pinchart: > > On Wednesday 30 Nov 2016 20:46:23 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 > > > > Ah, so it's for support of a future patch. You should explain that in the > > commit message. > > OK > > > The other question I asked was whether it would be possible to come up > > with a generic implementation of this function that would work on all > > architectures. At first sight the blackfin implementation is similar, but > > the sparc one is quite different. Why does it have to be arch-specific ? > > I don't know but I guess it is link to how memory is mmapped by the > architecture Well, how about trying to find out then ? :-) > >> By testing those two flags I avoid to define twince > >> get_fb_unmapped_area() if the architecture already > >> provide it. > >> > >>>> +unsigned long get_fb_unmapped_area(struct file *filp, > >>>> + unsigned long addr, unsigned long > >>>> len, > >>>> + unsigned long pgoff, unsigned long > >>>> flags) > >>>> +{ > >>>> + struct fb_info * const info = filp->private_data; > >>>> + unsigned long fb_size = PAGE_ALIGN(info->fix.smem_len); > >>>> + > >>>> + if (pgoff > fb_size || len > fb_size - pgoff) > >>>> + return -EINVAL; > >>>> + > >>>> + return (unsigned long)info->screen_base + pgoff; > >>>> +} > >>>> +#endif > >>>> + > >>>> static const struct file_operations fb_fops = { > >>>> .owner = THIS_MODULE, > >>>> .read = fb_read,
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
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) +unsigned long get_fb_unmapped_area(struct file *filp, + unsigned long addr, unsigned long len, + unsigned long pgoff, unsigned long flags) +{ + struct fb_info * const info = filp->private_data; + unsigned long fb_size = PAGE_ALIGN(info->fix.smem_len); + + if (pgoff > fb_size || len > fb_size - pgoff) + return -EINVAL; + + return (unsigned long)info->screen_base + pgoff; +} +#endif + static const struct file_operations fb_fops = { .owner = THIS_MODULE, .read = fb_read,
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(+)