On Mon, Apr 12, 2010 at 09:54:28AM +1000, Dave Airlie wrote:
> On Sat, 2010-04-10 at 21:55 +0200, marcin.slusarz at 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 at gmail.com>
> > Cc: Dave Airlie <airlied at redhat.com>
> > Cc: Peter Jones <pjones at redhat.com>
> > Cc: Andrew Morton <akpm at 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
>
>
