Menu
▾
▴
dri-devel
|
From: Felix <fx...@gm...> - 2004-09-06 14:52:59
|
Hi, after upgrading the DRM (it has been a while since the last cvs update) the Savage driver stopped working. I tracked it down to the DRM refusing to create multiple framebuffer-type mappings. If it finds an existing mappi= ng it returns it instead of creating a new one. However, the Savage driver needs multiple framebuffer-type mappings. The real frame buffer is supplemented by a so-called aperture where front/back/depth buffers can be accessed at fixed addresses (independent of their actual locations in the frame buffer) linearly even if the framebuffer is physically tiled. This behaviour is controlled by a set of tiled surface registers (IIRC). The HwMC code makes yet another framebuffer-type mapping. :-/ 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: =20 sets up the DRM to have the ability to have permanent maps while the driv= er is loaded... Is it really necessary to limit drivers to a single framebuffer-type mapping? Regards, Felix | Felix K=FChling <fx...@gm...> http://fxk.de.vu | | PGP Fingerprint: 6A3C 9566 5B30 DDED 73C3 B152 151C 5CC1 D888 E595 | |
|
From: Dave A. <ai...@li...> - 2004-09-06 22:23:37
|
> > 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 |
|
From: Jon S. <jon...@gm...> - 2004-09-06 23:26:04
|
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 <ai...@li...> 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 > > > ------------------------------------------------------- > 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_id=5047&alloc_id=10808&op=click > > > -- > _______________________________________________ > Dri-devel mailing list > Dri...@li... > https://lists.sourceforge.net/lists/listinfo/dri-devel > -- Jon Smirl jon...@gm... |
|
From: Dave A. <ai...@li...> - 2004-09-06 23:45:56
|
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 <ai...@li...> 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 > > > > > > ------------------------------------------------------- > > 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_id=5047&alloc_id=10808&op=click > > > > > > -- > > _______________________________________________ > > Dri-devel mailing list > > Dri...@li... > > https://lists.sourceforge.net/lists/listinfo/dri-devel > > > > > > -- David Airlie, Software Engineer http://www.skynet.ie/~airlied / airlied at skynet.ie pam_smb / Linux DECstation / Linux VAX / ILUG person |
|
From: Jon S. <jon...@gm...> - 2004-09-06 23:56:50
|
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. If not I can complicate the loop more to handle this case. On Tue, 7 Sep 2004 00:45:53 +0100 (IST), Dave Airlie <ai...@li...> 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 <ai...@li...> 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 > > > > > > > > > ------------------------------------------------------- > > > 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_id=5047&alloc_id=10808&op=click > > > > > > > > > -- > > > _______________________________________________ > > > Dri-devel mailing list > > > Dri...@li... > > > https://lists.sourceforge.net/lists/listinfo/dri-devel > > > > > > > > > > > > > -- > David Airlie, Software Engineer > http://www.skynet.ie/~airlied / airlied at skynet.ie > pam_smb / Linux DECstation / Linux VAX / ILUG person > > -- Jon Smirl jon...@gm... |
|
From: Felix <fx...@gm...> - 2004-09-07 11:21:49
|
On Mon, 6 Sep 2004 19:56:49 -0400 Jon Smirl <jon...@gm...> 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. >=20 > 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. >=20 > 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. >=20 >=20 > On Tue, 7 Sep 2004 00:45:53 +0100 (IST), Dave Airlie <ai...@li...> w= rote: > >=20 > > 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, > >=20 > > A permanently mapped framebuffer, shouldn't stop you adding another > > mapping for tiling/whatever... or should it.. > >=20 > > Dave. > >=20 > >=20 > >=20 > > On Mon, 6 Sep 2004, Jon Smirl wrote: > >=20 > > > 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 =3D DRM(initmap)( dev, pci_resource_start( dev->pd= ev, 2 ), > > > + pci_resource_len( dev->pdev, 2 ) / 2, _DRM_REGI= STERS, 0 ))) > > > + return retcode; > > > + > > > + /* framebuffer */ > > > + if( (retcode =3D DRM(initmap)( dev, pci_resource_start( dev->pd= ev, 0 ), > > > + pci_resource_len( dev->pdev, 0 ), _DRM_FRAME_BU= FFER, > > > _DRM_WRITE_COMBINING ))) > > > + return retcode; > > > > > > > > > > > > On Mon, 6 Sep 2004 23:23:27 +0100 (IST), Dave Airlie <airlied@linux.i= e> 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 whil= e the driver is loaded... > > > > > > > > > > Is it really necessary to limit drivers to a single framebuffer-t= ype > > > > > mapping? > > > > > > > > Just in case anyone is wondering this is why I can be a bit slow pu= shing > > > > 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 b= ack 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=FChling <fx...@gm...> http://fxk.de.vu | | PGP Fingerprint: 6A3C 9566 5B30 DDED 73C3 B152 151C 5CC1 D888 E595 | |
|
From: Jon S. <jon...@gm...> - 2004-09-07 00:16:15
Attachments:
perm.patch
|
I decided my loop was too fancy so I just deleted it and replaced it with a simple flag. If the driver uses permanent maps the flag gets set and addmaps from user space have no effect. If the driver doesn't use permanent maps everything should work the old way. If this looks good I can check it in. |
|
From: Felix <fx...@gm...> - 2004-09-07 11:21:49
|
On Mon, 6 Sep 2004 20:16:12 -0400 Jon Smirl <jon...@gm...> wrote: > I decided my loop was too fancy so I just deleted it and replaced it > with a simple flag. If the driver uses permanent maps the flag gets > set and addmaps from user space have no effect. If the driver doesn't > use permanent maps everything should work the old way. >=20 > If this looks good I can check it in. >=20 How would this work with an old DDX that still tries to call AddMap? Especially in the case where the DDX makes multiple maps of the same type? Felix | Felix K=FChling <fx...@gm...> http://fxk.de.vu | | PGP Fingerprint: 6A3C 9566 5B30 DDED 73C3 B152 151C 5CC1 D888 E595 | |
|
From: Jon S. <jon...@gm...> - 2004-09-07 00:23:09
|
On Mon, 6 Sep 2004 16:55:10 +0200, Felix K=FChling <fx...@gm...> wrote: > after upgrading the DRM (it has been a while since the last cvs update) > the Savage driver stopped working. I tracked it down to the DRM refusing > to create multiple framebuffer-type mappings. If it finds an existing map= ping > it returns it instead of creating a new one. However, the Savage driver > needs multiple framebuffer-type mappings. The real frame buffer is > supplemented by a so-called aperture where front/back/depth buffers can > be accessed at fixed addresses (independent of their actual locations in > the frame buffer) linearly even if the framebuffer is physically tiled. > This behaviour is controlled by a set of tiled surface registers (IIRC). > The HwMC code makes yet another framebuffer-type mapping. :-/ Does the Savage DRM driver know enough to make these extra mappings? Or is computation in user space required? Is it possible for the Savage DRMdriver to create the necessary maps using initmap like I did in the Radeon example? I was unaware of this feature of the Savage chipset when I made the changes and I don't own one to test with. Is there a representative PCI card available for this chipset? --=20 Jon Smirl jon...@gm... |
|
From: Jon S. <jon...@gm...> - 2004-09-07 15:40:03
Attachments:
perm.patch
|
Here's a new version that should be compatible with all of the cases: 1) multiple maps 2) old X doing AddMap 3) fixed X doing GetMap - X needs to switch to this after all the drivers get fixed 4) drivers that have implemented permanent maps 4) drivers that have not implemented permanent maps I have a fix for my editor now that strips trailing blanks so I'm generating a few extra diffs but this will get rid of the trailing blanks in CVS after a few check ins. The reason for this change is to prevent X from moving the address of the framebuffer in PCI space. That is an illegal thing for X to do on Linux without also making the appropriate kernel calls. After all of the DRM drivers implement permanent mapping, X will get an error if it tried to move the framebuffer address. |
|
From: Felix <fx...@gm...> - 2004-09-07 21:05:59
|
Thanks. This reenables 3D accel on the Savage. I havn't tested it on any other driver. But the patch looks good to me. Regards, Felix On Tue, 7 Sep 2004 11:39:59 -0400 Jon Smirl <jon...@gm...> wrote: > Here's a new version that should be compatible with all of the cases: > 1) multiple maps > 2) old X doing AddMap > 3) fixed X doing GetMap - X needs to switch to this after all the > drivers get fixed > 4) drivers that have implemented permanent maps > 4) drivers that have not implemented permanent maps >=20 > I have a fix for my editor now that strips trailing blanks so I'm > generating a few extra diffs but this will get rid of the trailing > blanks in CVS after a few check ins. >=20 > The reason for this change is to prevent X from moving the address of > the framebuffer in PCI space. That is an illegal thing for X to do on > Linux without also making the appropriate kernel calls. After all of > the DRM drivers implement permanent mapping, X will get an error if it > tried to move the framebuffer address. >=20 | Felix K=FChling <fx...@gm...> http://fxk.de.vu | | PGP Fingerprint: 6A3C 9566 5B30 DDED 73C3 B152 151C 5CC1 D888 E595 | |
|
From: Jon S. <jon...@gm...> - 2004-09-07 21:36:03
|
I'd be interested to know if you can get permanent maps working in the Savage driver. I sent the example of how the radeon does it. On Tue, 7 Sep 2004 23:08:00 +0200, Felix K=FChling <fx...@gm...> wrote: > Thanks. This reenables 3D accel on the Savage. I havn't tested it on any > other driver. But the patch looks good to me. >=20 > Regards, > Felix >=20 > On Tue, 7 Sep 2004 11:39:59 -0400 >=20 >=20 > Jon Smirl <jon...@gm...> wrote: >=20 > > Here's a new version that should be compatible with all of the cases: > > 1) multiple maps > > 2) old X doing AddMap > > 3) fixed X doing GetMap - X needs to switch to this after all the > > drivers get fixed > > 4) drivers that have implemented permanent maps > > 4) drivers that have not implemented permanent maps > > > > I have a fix for my editor now that strips trailing blanks so I'm > > generating a few extra diffs but this will get rid of the trailing > > blanks in CVS after a few check ins. > > > > The reason for this change is to prevent X from moving the address of > > the framebuffer in PCI space. That is an illegal thing for X to do on > > Linux without also making the appropriate kernel calls. After all of > > the DRM drivers implement permanent mapping, X will get an error if it > > tried to move the framebuffer address. > > >=20 >=20 > | Felix K=FChling <fx...@gm...> http://fxk.de.vu | > | PGP Fingerprint: 6A3C 9566 5B30 DDED 73C3 B152 151C 5CC1 D888 E595 | >=20 --=20 Jon Smirl jon...@gm... |
|
From: David <de...@on...> - 2004-09-08 15:08:05
|
El Martes, 7 de Septiembre del 2004 11:08 PM, Felix K=FChling escribi=F3: > Thanks. This reenables 3D accel on the Savage. I havn't tested it on any > other driver. But the patch looks good to me. Tested on Savage4. It works. |
|
From: Keith W. <ke...@tu...> - 2004-09-07 15:59:20
|
Jon Smirl wrote: > Here's a new version that should be compatible with all of the cases: > 1) multiple maps > 2) old X doing AddMap > 3) fixed X doing GetMap - X needs to switch to this after all the > drivers get fixed > 4) drivers that have implemented permanent maps > 4) drivers that have not implemented permanent maps > > I have a fix for my editor now that strips trailing blanks so I'm > generating a few extra diffs but this will get rid of the trailing > blanks in CVS after a few check ins. Ugh. If we're going to do whitespace changes can we do them seperately to "proper" changes, please? Keith |
|
From: Jon S. <jon...@gm...> - 2004-09-07 16:09:24
Attachments:
perm.patch
|
New version of same patch without the whitespace clean up On Tue, 07 Sep 2004 16:59:09 +0100, Keith Whitwell <ke...@tu...> wrote: > Ugh. If we're going to do whitespace changes can we do them seperately to > "proper" changes, please? > > Keith > -- Jon Smirl jon...@gm... |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X