lkml.org 
[lkml]   [2010]   [Apr]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/3] fbmem: fix aperture overlapping check
On Mon, Apr 12, 2010 at 09:54:28AM +1000, Dave Airlie wrote:
> On Sat, 2010-04-10 at 21:55 +0200, marcin.slusarz@gmail.com wrote:
> > fb_do_apertures_overlap is returning wrong value when one aperture
> > is completely whithin the other. Add generic ranges_overlap macro
> > (probably kernel.h candidate) and use it here.
> >
>
> That doesn't seem right.
>
> The rules are:
>
> the generic aperture has to be equal or smaller than the hw aperture,
> otherwise the generic driver will be trashing random hw pieces on the
> machine.

Why "it has to"? Why generic aperture can't start one byte before hw?

> So with that in mind, the check makes sure the generic aperture starts
> somewhere inside the hw aperture, which the test clearly gets right.

So your only objection is that it's impossible with the current code?

> Have you got a pointer to a machine where it fails?

No, it failed with an artifical test while I was working on vga16fb handoff
(unfinished).

> Dave.
>
>
> > Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Peter Jones <pjones@redhat.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > ---
> > drivers/video/fbmem.c | 24 +++++++++++++++++-------
> > 1 files changed, 17 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> > index a15b44e..41f2e5e 100644
> > --- a/drivers/video/fbmem.c
> > +++ b/drivers/video/fbmem.c
> > @@ -1468,15 +1468,25 @@ static int fb_check_foreignness(struct fb_info *fi)
> > return 0;
> > }
> >
> > +/**
> > + * ranges_overlap - check whether two ranges overlap (their intersection is not empty)
> > + * @start1: start of the first range
> > + * @size1: length of the first range
> > + * @start2: start of the second range
> > + * @size2: length of the second range
> > + */
> > +#define ranges_overlap(start1, size1, start2, size2) ({ \
> > + typeof(start1) __start1 = (start1); \
> > + typeof(size1) __size1 = (size1); \
> > + typeof(start2) __start2 = (start2); \
> > + typeof(size2) __size2 = (size2); \
> > + __start1 < __start2 + __size2 && __start1 + __size1 > __start2; \
> > +})
> > +
> > static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
> > {
> > - /* is the generic aperture base the same as the HW one */
> > - if (gen->aperture_base == hw->aperture_base)
> > - return true;
> > - /* is the generic aperture base inside the hw base->hw base+size */
> > - if (gen->aperture_base > hw->aperture_base && gen->aperture_base <= hw->aperture_base + hw->aperture_size)
> > - return true;
> > - return false;
> > + return ranges_overlap(gen->aperture_base, gen->aperture_size,
> > + hw->aperture_base, hw->aperture_size);
> > }
> > /**
> > * register_framebuffer - registers a frame buffer device
>
>


\
 
 \ /
  Last update: 2010-04-12 13:43    [from the cache]
©2003-2011 Jasper Spaans