On Mon, 6 Sep 2004 19:56:49 -0400
Jon Smirl <[EMAIL PROTECTED]> wrote:

> AddMap has the loop to find the existing mapping and replace it.
> initmap doesn't have that loop so it allows multiple adds. I wanted to
> just make AddMap refuse a REG/FB map request but I was trying to be
> compatible while we changed the drivers over.
> 
> Can we just switch the drivers over right now and I'll make AddMap
> ignore requests to change the mappings? It's only a couple of line of
> code in each driver, but the driver maintainers need to tell us which
> PCI resource numbers to map. I can switch radeon/r128 right now if we
> are ready.

I'm not sure it's that simple. The way I know DRI policy WRT binary
compatibility you'd have to make sure that the new DRM still works with
old DDX drivers. So it's fine to generate the maps in the DRM driver if
there is enough info. Old DDX drivers still calling AddMap must be
returned the correct handles to the maps, even if they call AddMap
multiple times with the same map type. The problem with savage was that
it alway got the handle of the first frambuffer map. This got handed
over to the 3D driver. When the 3D driver tried to map the second
framebuffer mapping to its own address space the call to DRM(map)
returned -EINVAL because the address range indicated by the 3D driver
didn't match the size of the map.

> 
> If not I can complicate the loop more to handle this case.

I'd vote for making AddMap search for an identical map (same map type,
size and offset). If none exists then a new one should be created.

This is with my still very limited understanding of the DRM. I'm open to
other suggestions.

> 
> 
> On Tue, 7 Sep 2004 00:45:53 +0100 (IST), Dave Airlie <[EMAIL PROTECTED]> wrote:
> > 
> > But does the new code deal with multiple mappings.. the radeon doesn't do
> > this .... at the moment they call multiple addmaps for the framebuffer,
> > 
> > A permanently mapped framebuffer, shouldn't stop you adding another
> > mapping for tiling/whatever... or should it..
> > 
> > Dave.
> > 
> > 
> > 
> > On Mon, 6 Sep 2004, Jon Smirl wrote:
> > 
> > > The plan for the addMap changes is to get rid of the loop where the
> > > user space code asks the driver where the resources are and then sets
> > > those values back into the driver. Since the driver already knows
> > > these values it should just create the maps itself. This removes the
> > > possibility of the user space code changing those values before
> > > passing them back.
> > >
> > > This is the code radeon driver uses to create the permanent resource
> > > mappings.  Adding the corresponding code the the savage driver will
> > > fix the problem. initmap won't stop you from making more than one
> > > mapping.
> > >
> > > I can look into fixing addmap, but if you switch to having the driver
> > > initmap for REGISTERS/ FRAME_BUFFER you won't need to addmap them from
> > > user space and it won't matter if the call fails.
> > >
> > > +
> > > +     /* registers */
> > > +     /* PCI space is twice the real size, so that you can have a RW and
> > > RO mapping */
> > > +     if( (retcode = DRM(initmap)( dev, pci_resource_start( dev->pdev, 2 ),
> > > +                     pci_resource_len( dev->pdev, 2 ) / 2, _DRM_REGISTERS, 0 )))
> > > +             return retcode;
> > > +
> > > +     /* framebuffer */
> > > +     if( (retcode = DRM(initmap)( dev, pci_resource_start( dev->pdev, 0 ),
> > > +                     pci_resource_len( dev->pdev, 0 ), _DRM_FRAME_BUFFER,
> > > _DRM_WRITE_COMBINING )))
> > > +             return retcode;
> > >
> > >
> > >
> > > On Mon, 6 Sep 2004 23:23:27 +0100 (IST), Dave Airlie <[EMAIL PROTECTED]> wrote:
> > > >
> > > > >
> > > > > Anyway, I suspect the behaviour of DRM(addmap) changed recently. The
> > > > > only addmap-related comment I could find on dri-patches is this:
> > > > >
> > > > >   addmap-base-2 patch from Jon Smirl:
> > > > >
> > > > >   sets up the DRM to have the ability to have permanent maps while the 
> > > > > driver is loaded...
> > > > >
> > > > > Is it really necessary to limit drivers to a single framebuffer-type
> > > > > mapping?
> > > >
> > > > Just in case anyone is wondering this is why I can be a bit slow pushing
> > > > to Linus, finding these issues takes time... this patch looked okay to me
> > > > but I never knew what the savage was up to ...
> > > >
> > > > I don't think there is any reason to limit it to only one mapping,
> > > >
> > > > Hopefully Jon can think of a way around it, you should be able to back out
> > > > that change on your system for now until Jon gets online..
> > > >
> > > > Dave.
> > > >
> > > > --
> > > > David Airlie, Software Engineer
> > > > http://www.skynet.ie/~airlied / airlied at skynet.ie
> > > > pam_smb / Linux DECstation / Linux VAX / ILUG person
> > > >
> > > >


| Felix Kühling <[EMAIL PROTECTED]>                     http://fxk.de.vu |
| PGP Fingerprint: 6A3C 9566 5B30 DDED 73C3  B152 151C 5CC1 D888 E595 |


-------------------------------------------------------
This SF.Net email is sponsored by BEA Weblogic Workshop
FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_idP47&alloc_id808&op=click
--
_______________________________________________
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to