Re: [PATCH]: drm: cleanup DRM_DEBUG() parameters
From: Márton Németh [EMAIL PROTECTED]
As DRM_DEBUG macro already prints out the __FUNCTION__ string (see
drivers/char/drm/drmP.h), it is not worth doing this again. At some
other places the ending \n was added.
Signed-off-by: Márton Németh [EMAIL PROTECTED]
---
diff -uprN linux-2.6.23-mm1.orig/drivers/char/drm/ati_pcigart.c
linux-2.6.23-mm1/drivers/char/drm/ati_pcigart.c
--- linux-2.6.23-mm1.orig/drivers/char/drm/ati_pcigart.c2007-10-09
22:31:38.0 +0200
+++ linux-2.6.23-mm1/drivers/char/drm/ati_pcigart.c 2007-10-17
22:22:19.0 +0200
@@ -41,7 +41,7 @@ static void *drm_ati_alloc_pcigart_table
struct page *page;
int i;
- DRM_DEBUG(%s: alloc %d order\n, __FUNCTION__, order);
+ DRM_DEBUG(%d order\n, order);
address = __get_free_pages(GFP_KERNEL | __GFP_COMP,
order);
@@ -54,7 +54,7 @@ static void *drm_ati_alloc_pcigart_table
for (i = 0; i order; i++, page++)
SetPageReserved(page);
- DRM_DEBUG(%s: returning 0x%08lx\n, __FUNCTION__, address);
+ DRM_DEBUG(returning 0x%08lx\n, address);
return (void *)address;
}
@@ -63,7 +63,7 @@ static void drm_ati_free_pcigart_table(v
struct page *page;
int i;
int num_pages = 1 order;
- DRM_DEBUG(%s\n, __FUNCTION__);
+ DRM_DEBUG(\n);
page = virt_to_page((unsigned long)address);
diff -uprN linux-2.6.23-mm1.orig/drivers/char/drm/drm_irq.c
linux-2.6.23-mm1/drivers/char/drm/drm_irq.c
--- linux-2.6.23-mm1.orig/drivers/char/drm/drm_irq.c2007-10-17
22:19:21.0 +0200
+++ linux-2.6.23-mm1/drivers/char/drm/drm_irq.c 2007-10-17 22:22:19.0
+0200
@@ -107,7 +107,7 @@ static int drm_irq_install(struct drm_de
dev-irq_enabled = 1;
mutex_unlock(dev-struct_mutex);
- DRM_DEBUG(%s: irq=%d\n, __FUNCTION__, dev-irq);
+ DRM_DEBUG(irq=%d\n, dev-irq);
if (drm_core_check_feature(dev, DRIVER_IRQ_VBL)) {
init_waitqueue_head(dev-vbl_queue);
@@ -164,7 +164,7 @@ int drm_irq_uninstall(struct drm_device
if (!irq_enabled)
return -EINVAL;
- DRM_DEBUG(%s: irq=%d\n, __FUNCTION__, dev-irq);
+ DRM_DEBUG(irq=%d\n, dev-irq);
dev-driver-irq_uninstall(dev);
diff -uprN linux-2.6.23-mm1.orig/drivers/char/drm/drm_scatter.c
linux-2.6.23-mm1/drivers/char/drm/drm_scatter.c
--- linux-2.6.23-mm1.orig/drivers/char/drm/drm_scatter.c2007-10-17
22:19:21.0 +0200
+++ linux-2.6.23-mm1/drivers/char/drm/drm_scatter.c 2007-10-17
22:31:42.0 +0200
@@ -67,7 +67,7 @@ int drm_sg_alloc(struct drm_device *dev,
struct drm_sg_mem *entry;
unsigned long pages, i, j;
- DRM_DEBUG(%s\n, __FUNCTION__);
+ DRM_DEBUG(\n);
if (!drm_core_check_feature(dev, DRIVER_SG))
return -EINVAL;
@@ -81,7 +81,7 @@ int drm_sg_alloc(struct drm_device *dev,
memset(entry, 0, sizeof(*entry));
pages = (request-size + PAGE_SIZE - 1) / PAGE_SIZE;
- DRM_DEBUG(sg size=%ld pages=%ld\n, request-size, pages);
+ DRM_DEBUG(size=%ld pages=%ld\n, request-size, pages);
entry-pages = pages;
entry-pagelist = drm_alloc(pages * sizeof(*entry-pagelist),
@@ -122,8 +122,8 @@ int drm_sg_alloc(struct drm_device *dev,
entry-handle = ScatterHandle((unsigned long)entry-virtual);
- DRM_DEBUG(sg alloc handle = %08lx\n, entry-handle);
- DRM_DEBUG(sg alloc virtual = %p\n, entry-virtual);
+ DRM_DEBUG(handle = %08lx\n, entry-handle);
+ DRM_DEBUG(virtual = %p\n, entry-virtual);
for (i = (unsigned long)entry-virtual, j = 0; j pages;
i += PAGE_SIZE, j++) {
@@ -210,7 +210,7 @@ int drm_sg_free(struct drm_device *dev,
if (!entry || entry-handle != request-handle)
return -EINVAL;
- DRM_DEBUG(sg free virtual = %p\n, entry-virtual);
+ DRM_DEBUG(virtual = %p\n, entry-virtual);
drm_sg_cleanup(entry);
diff -uprN linux-2.6.23-mm1.orig/drivers/char/drm/drm_vm.c
linux-2.6.23-mm1/drivers/char/drm/drm_vm.c
--- linux-2.6.23-mm1.orig/drivers/char/drm/drm_vm.c 2007-10-17
22:19:21.0 +0200
+++ linux-2.6.23-mm1/drivers/char/drm/drm_vm.c 2007-10-17 22:22:19.0
+0200
@@ -180,7 +180,7 @@ static __inline__ struct page *drm_do_vm
return NOPAGE_SIGBUS;
get_page(page);
- DRM_DEBUG(shm_nopage 0x%lx\n, address);
+ DRM_DEBUG(0x%lx\n, address);
return page;
}
@@ -294,7 +294,7 @@ static __inline__ struct page *drm_do_vm
get_page(page);
- DRM_DEBUG(dma_nopage 0x%lx (page %lu)\n, address, page_nr);
+ DRM_DEBUG(0x%lx (page %lu)\n, address, page_nr);
return page;
}
diff -uprN linux-2.6.23-mm1.orig/drivers/char/drm/i810_dma.c
linux-2.6.23-mm1/drivers/char/drm/i810_dma.c
--- linux-2.6.23-mm1.orig/drivers/char/drm/i810_dma.c 2007-10-17
22:19:21.0 +0200
+++
Re: [PATCH]: drm: cleanup DRM_DEBUG() parameters
From: Márton Németh [EMAIL PROTECTED]
As DRM_DEBUG macro already prints out the __FUNCTION__ string (see
drivers/char/drm/drmP.h), it is not worth doing this again. At some
other places the ending \n was added.
Signed-off-by: Márton Németh [EMAIL PROTECTED]
---
diff -uprN linux-2.6.23-mm1.orig/drivers/char/drm/ati_pcigart.c
linux-2.6.23-mm1/drivers/char/drm/ati_pcigart.c
--- linux-2.6.23-mm1.orig/drivers/char/drm/ati_pcigart.c2007-10-09
22:31:38.0 +0200
+++ linux-2.6.23-mm1/drivers/char/drm/ati_pcigart.c 2007-10-17
22:22:19.0 +0200
@@ -41,7 +41,7 @@ static void *drm_ati_alloc_pcigart_table
struct page *page;
int i;
- DRM_DEBUG(%s: alloc %d order\n, __FUNCTION__, order);
+ DRM_DEBUG(%d order\n, order);
address = __get_free_pages(GFP_KERNEL | __GFP_COMP,
order);
@@ -54,7 +54,7 @@ static void *drm_ati_alloc_pcigart_table
for (i = 0; i order; i++, page++)
SetPageReserved(page);
- DRM_DEBUG(%s: returning 0x%08lx\n, __FUNCTION__, address);
+ DRM_DEBUG(returning 0x%08lx\n, address);
return (void *)address;
}
@@ -63,7 +63,7 @@ static void drm_ati_free_pcigart_table(v
struct page *page;
int i;
int num_pages = 1 order;
- DRM_DEBUG(%s\n, __FUNCTION__);
+ DRM_DEBUG(\n);
page = virt_to_page((unsigned long)address);
diff -uprN linux-2.6.23-mm1.orig/drivers/char/drm/drm_irq.c
linux-2.6.23-mm1/drivers/char/drm/drm_irq.c
--- linux-2.6.23-mm1.orig/drivers/char/drm/drm_irq.c2007-10-17
22:19:21.0 +0200
+++ linux-2.6.23-mm1/drivers/char/drm/drm_irq.c 2007-10-17 22:22:19.0
+0200
@@ -107,7 +107,7 @@ static int drm_irq_install(struct drm_de
dev-irq_enabled = 1;
mutex_unlock(dev-struct_mutex);
- DRM_DEBUG(%s: irq=%d\n, __FUNCTION__, dev-irq);
+ DRM_DEBUG(irq=%d\n, dev-irq);
if (drm_core_check_feature(dev, DRIVER_IRQ_VBL)) {
init_waitqueue_head(dev-vbl_queue);
@@ -164,7 +164,7 @@ int drm_irq_uninstall(struct drm_device
if (!irq_enabled)
return -EINVAL;
- DRM_DEBUG(%s: irq=%d\n, __FUNCTION__, dev-irq);
+ DRM_DEBUG(irq=%d\n, dev-irq);
dev-driver-irq_uninstall(dev);
diff -uprN linux-2.6.23-mm1.orig/drivers/char/drm/drm_scatter.c
linux-2.6.23-mm1/drivers/char/drm/drm_scatter.c
--- linux-2.6.23-mm1.orig/drivers/char/drm/drm_scatter.c2007-10-17
22:19:21.0 +0200
+++ linux-2.6.23-mm1/drivers/char/drm/drm_scatter.c 2007-10-17
22:31:42.0 +0200
@@ -67,7 +67,7 @@ int drm_sg_alloc(struct drm_device *dev,
struct drm_sg_mem *entry;
unsigned long pages, i, j;
- DRM_DEBUG(%s\n, __FUNCTION__);
+ DRM_DEBUG(\n);
if (!drm_core_check_feature(dev, DRIVER_SG))
return -EINVAL;
@@ -81,7 +81,7 @@ int drm_sg_alloc(struct drm_device *dev,
memset(entry, 0, sizeof(*entry));
pages = (request-size + PAGE_SIZE - 1) / PAGE_SIZE;
- DRM_DEBUG(sg size=%ld pages=%ld\n, request-size, pages);
+ DRM_DEBUG(size=%ld pages=%ld\n, request-size, pages);
entry-pages = pages;
entry-pagelist = drm_alloc(pages * sizeof(*entry-pagelist),
@@ -122,8 +122,8 @@ int drm_sg_alloc(struct drm_device *dev,
entry-handle = ScatterHandle((unsigned long)entry-virtual);
- DRM_DEBUG(sg alloc handle = %08lx\n, entry-handle);
- DRM_DEBUG(sg alloc virtual = %p\n, entry-virtual);
+ DRM_DEBUG(handle = %08lx\n, entry-handle);
+ DRM_DEBUG(virtual = %p\n, entry-virtual);
for (i = (unsigned long)entry-virtual, j = 0; j pages;
i += PAGE_SIZE, j++) {
@@ -210,7 +210,7 @@ int drm_sg_free(struct drm_device *dev,
if (!entry || entry-handle != request-handle)
return -EINVAL;
- DRM_DEBUG(sg free virtual = %p\n, entry-virtual);
+ DRM_DEBUG(virtual = %p\n, entry-virtual);
drm_sg_cleanup(entry);
diff -uprN linux-2.6.23-mm1.orig/drivers/char/drm/drm_vm.c
linux-2.6.23-mm1/drivers/char/drm/drm_vm.c
--- linux-2.6.23-mm1.orig/drivers/char/drm/drm_vm.c 2007-10-17
22:19:21.0 +0200
+++ linux-2.6.23-mm1/drivers/char/drm/drm_vm.c 2007-10-17 22:22:19.0
+0200
@@ -180,7 +180,7 @@ static __inline__ struct page *drm_do_vm
return NOPAGE_SIGBUS;
get_page(page);
- DRM_DEBUG(shm_nopage 0x%lx\n, address);
+ DRM_DEBUG(0x%lx\n, address);
return page;
}
@@ -294,7 +294,7 @@ static __inline__ struct page *drm_do_vm
get_page(page);
- DRM_DEBUG(dma_nopage 0x%lx (page %lu)\n, address, page_nr);
+ DRM_DEBUG(0x%lx (page %lu)\n, address, page_nr);
return page;
}
diff -uprN linux-2.6.23-mm1.orig/drivers/char/drm/i810_dma.c
linux-2.6.23-mm1/drivers/char/drm/i810_dma.c
--- linux-2.6.23-mm1.orig/drivers/char/drm/i810_dma.c 2007-10-17
22:19:21.0 +0200
+++
Re: [patch] post superioctl inteface removal.
Hi, Eric. Eric Anholt wrote: On Wed, 2007-10-17 at 11:32 +0200, Thomas Hellström wrote: Dave Airlie wrote: DRM_BO_HINT_DONT_FENCE is implied, and use that instead of the set pin interface. We can perhaps rename it to drmBOSetStatus or something more suitable. This will get rid of the user-space unfenced list access (which I believe was the main motivation behind the set pin interface?) while keeping the currently heavily used (at least in Poulsbo) functionality to move out NO_EVICT scanout buffers to local memory before unpinning them, (to avoid VRAM and TT fragmentation, as DRI clients may still reference those buffers, so they won't get destroyed before a new one is allocated). Yes, the unfenced list thing and the associated DRM_BO_HINTS required was part of the motivation for set_pin. But it also made sense from the X Server's perspective: we want to allocate buffers, and then there are times where we want them pinned because we're scanning out from them, but that pinning isn't associated with doing rendering to/from them that requires fencing as part of being involved in the command stream. A good point that I missed. If we go for setStatus() we should remove all fencing parameters, making it basically a setPin() with flags. And it avoided the issue with the previous validate-based interface where if you didn't sync the flags with what the X Server had updated, you might mistakenly pin/unpin a buffer. This was actually a bug in the code, which is now fixed by only allowing the creator of a shared buffer to change the buffer mask. Perhaps we should change this to only allow the creator to change pinning flags. Can you clarify the operation being done where you move scanout buffers before unpinning them? That seems contradictory to me -- how are you scanning out while the object is being moved, and how are you considering it pinned during that time? Actually it's very similar to Dave's issue, and the buffers aren't pinned when they are thrown out. What we'd want to do is the following: 1) Turn of Crtcs. 2) Unpin the buffer 3) Destroy the buffer, leaving it's memory area free. 4) Create and pin new buffer (skipping the copy) 5) Turn on Crtcs. However, with DRI clients operating, 3) will fail. As they may maintain a reference on the front buffer, the old buffer won't immediately get destroyed and it's aperture / VRAM memory area isn't freed up, unless it gets evicted by a new allocation. This will in many cases lead to fragmentation where it is really possible to avoid it. The best thing we can do at 3) is to move it out, and then unreference it. When the DRI client recognizes through the SAREA that there's a new front buffer, it will immediately release its reference on the old one, but basically, the old front buffer may be hanging around for quite some time (paused dri clients...) and we don't want it to be present in the aperture, even if it's evictable. This won't stop fragmentation in all cases, but will certainly reduce it. Potentially related to your concerns, dave brought up an issue with the current set_pin implementation. Take the framebuffer resize sequence I'd proposed before: Turn off CRTCs Unpin old framebuffer Allocate new framebuffer Copy from old to new Free old Pin new Turn it back on. We'll have chosen an offset for new at Copy old from new, and so during pin we'll just pin it right in place without moving it to avoid fragmentation. This sucks. It was a problem with the old interface as well, but it should be fixable by replacing the current poorly-suited validate call in the set_pin implementation with something that chooses the best available offset rather than just anything that matches memory types, and moves it into place before pinning. Moving out the old one before allocating a new one will reduce this problem but not eliminate it. The low level memory manager chooses the best region within a single memory type, but we currently don't optimize between memory types. The meaning of best is that it chooses the smallest free region where the buffer will fit. It would also allow us to specify where we want to pin buffers. If we remove the memory flag specification from drmBOCreate there's no other way to do that, except running the buffer through a superioctl which isn't very nice. Yeah, not making bo_set_pin include a pin location was an oversight that I didn't end up correcting before push. Also it would make it much easier to unbreak i915 zone rendering and derived work. If we can agree on this, I'll come up with a patch. Have you had a chance to look at this I can probably spend some time on this to get the interface finalised.. Dave. Hi, Dave, So, I did some quick work on this with the result in the drm-ttm-finalize branch. Basically what's done is to revert the setPin interface, and replace drmBOValidate with drmBOSetStatus. drmBOSetStatus is a
Re: [patch] post superioctl inteface removal.
Thomas Hellström wrote: Hi, Eric. Eric Anholt wrote: ... Can you clarify the operation being done where you move scanout buffers before unpinning them? That seems contradictory to me -- how are you scanning out while the object is being moved, and how are you considering it pinned during that time? Actually it's very similar to Dave's issue, and the buffers aren't pinned when they are thrown out. What we'd want to do is the following: 1) Turn of Crtcs. 2) Unpin the buffer 3) Destroy the buffer, leaving it's memory area free. 4) Create and pin new buffer (skipping the copy) 5) Turn on Crtcs. However, with DRI clients operating, 3) will fail. As they may maintain a reference on the front buffer, the old buffer won't immediately get destroyed and it's aperture / VRAM memory area isn't freed up, unless it gets evicted by a new allocation. Is there really a long-term need for DRI clients to maintain a reference to the front buffer? We're moving away from this in lot of ways and if it is a benefit to the TTM work, we could look at severing that tie sooner rather than later... This will in many cases lead to fragmentation where it is really possible to avoid it. The best thing we can do at 3) is to move it out, and then unreference it. When the DRI client recognizes through the SAREA that there's a new front buffer, it will immediately release its reference on the old one, but basically, the old front buffer may be hanging around for quite some time (paused dri clients...) and we don't want it to be present in the aperture, even if it's evictable. This won't stop fragmentation in all cases, but will certainly reduce it. At very least, current DRI/ttm clients could be modified to only use the frontbuffer reference in locked regions, and to have some way of getting the correct handle for the current frontbuffer at that point. Longer term, it's easy to imagine DRI clients not touching the front buffer independently and not requiring a reference to that buffer... Keith - This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now http://get.splunk.com/ -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [patch] post superioctl inteface removal.
Keith Whitwell wrote: Thomas Hellström wrote: Hi, Eric. Eric Anholt wrote: ... Can you clarify the operation being done where you move scanout buffers before unpinning them? That seems contradictory to me -- how are you scanning out while the object is being moved, and how are you considering it pinned during that time? Actually it's very similar to Dave's issue, and the buffers aren't pinned when they are thrown out. What we'd want to do is the following: 1) Turn of Crtcs. 2) Unpin the buffer 3) Destroy the buffer, leaving it's memory area free. 4) Create and pin new buffer (skipping the copy) 5) Turn on Crtcs. However, with DRI clients operating, 3) will fail. As they may maintain a reference on the front buffer, the old buffer won't immediately get destroyed and it's aperture / VRAM memory area isn't freed up, unless it gets evicted by a new allocation. Is there really a long-term need for DRI clients to maintain a reference to the front buffer? We're moving away from this in lot of ways and if it is a benefit to the TTM work, we could look at severing that tie sooner rather than later... This will in many cases lead to fragmentation where it is really possible to avoid it. The best thing we can do at 3) is to move it out, and then unreference it. When the DRI client recognizes through the SAREA that there's a new front buffer, it will immediately release its reference on the old one, but basically, the old front buffer may be hanging around for quite some time (paused dri clients...) and we don't want it to be present in the aperture, even if it's evictable. This won't stop fragmentation in all cases, but will certainly reduce it. At very least, current DRI/ttm clients could be modified to only use the frontbuffer reference in locked regions, and to have some way of getting the correct handle for the current frontbuffer at that point. Longer term, it's easy to imagine DRI clients not touching the front buffer independently and not requiring a reference to that buffer... Doesn't Kristian changes to DRI interface (DRI2) already allow to clients to not care about front buffer. I mean if they all got private back buffer then they render into it. But i might have misunderstood this. Cheers, Jerome Glisse - This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now http://get.splunk.com/ -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [patch] post superioctl inteface removal.
Keith Whitwell wrote: Jerome Glisse wrote: Keith Whitwell wrote: Thomas Hellström wrote: Hi, Eric. Eric Anholt wrote: ... Can you clarify the operation being done where you move scanout buffers before unpinning them? That seems contradictory to me -- how are you scanning out while the object is being moved, and how are you considering it pinned during that time? Actually it's very similar to Dave's issue, and the buffers aren't pinned when they are thrown out. What we'd want to do is the following: 1) Turn of Crtcs. 2) Unpin the buffer 3) Destroy the buffer, leaving it's memory area free. 4) Create and pin new buffer (skipping the copy) 5) Turn on Crtcs. However, with DRI clients operating, 3) will fail. As they may maintain a reference on the front buffer, the old buffer won't immediately get destroyed and it's aperture / VRAM memory area isn't freed up, unless it gets evicted by a new allocation. Is there really a long-term need for DRI clients to maintain a reference to the front buffer? We're moving away from this in lot of ways and if it is a benefit to the TTM work, we could look at severing that tie sooner rather than later... While this is probably a good idea, the discussion is really whether we should use a more general drmBOSetStatus() or a subset drmBOSetPin(). With drmBOSetStatus() we can easily implement a workaround for the (perhaps soon disappearing) fragmentation problem, but there are a couple of other uses as well. /Thomas - This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now http://get.splunk.com/ -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [patch] post superioctl inteface removal.
On Thu, 2007-10-18 at 07:55 +0800, Keith Packard wrote: On Wed, 2007-10-17 at 16:40 -0700, Eric Anholt wrote: Turn off CRTCs Unpin old framebuffer Allocate new framebuffer Copy from old to new We needn't copy on resize, leaving us with allocate new, unpin old, pin new, free old. Seems like this would avoid some of the worst issues. True. The issue would still exist if you happened to accelerated clear your new frontbuffer before pinning it, which we could avoid in the driver. And even if you do everything right, in the case of expanding your framebuffer I don't think there would be any guarantee of getting put at the least-fragmenting place to have a pinned buffer. I think the solution to that that I suggested is reasonable and deals with thomas's and our fragmentation concerns (don't use validate, just find the space we would most like to be at not containing pinned objects, evict whoever's there, and pin it). Also, while I'm not a fan of the vague name of drmBOSetStatus, it sounds like it would be usable to do the map hinting I suggested. -- Eric Anholt [EMAIL PROTECTED] [EMAIL PROTECTED] [EMAIL PROTECTED] signature.asc Description: This is a digitally signed message part - This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now http://get.splunk.com/-- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [patch] post superioctl inteface removal.
On 10/18/07, Keith Whitwell [EMAIL PROTECTED] wrote: ... Doesn't Kristian changes to DRI interface (DRI2) already allow to clients to not care about front buffer. I mean if they all got private back buffer then they render into it. But i might have misunderstood this. Yes, of course. I'm not sure how watertight the isolation of the frontbuffer is in DRI2, but if it's complete and if the rule is that ttm supports only DRI2 clients, this concern would seem to be addressed. What I'm planning to do is to keep the front buffer bo handle in the kernel, associateed with the drm_drawable_t. The swap buffer ioctl will reference a drm_drawable_t as the front buffer destination and thus it will always be up to date and userspace won't hold any references to front buffers. The kernel holds a reference through each of the drm_drawable_t's that are non-redirected child-windows of the front buffer, and once the X server has re-attached the new front buffer bo to each drm_drawable_t, there should be no references to the front buffer. This happens in the SetWindowPixmap hook. It sounds like we want to free the old front buffer before we allocate the new one, in which case we'll have to call the SetWindowPixmap hook with a null pixmap or something to drop the kernel side references before allocating the new buffer. However, I was hoping we could avoid this, and allocate a new buffer while still scanning out from the old one, copy the contents, change the scan out address and then free the old one. This avoids flicker, and as for the fragmentation concern; can't we just alternate between allocating from different ends of the aperture? Or alternatively, do a two-step operation: allocate a temporary front buffer far enough into the aperture that we free the old buffer and then allocate the final new frontbuffer at offset 0? We should avoid turning off the crtcs here if at all possible. cheers, Kristian - This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now http://get.splunk.com/ -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
[RFC] full suspend/resume support for i915 DRM driver
We seem to see a lot of bug reports along the lines of, my machine
resumes but I can't see X or, I can see X but only with a bright
flashlight, etc. These sorts of problems are due to the fact that
the X server isn't designed to do full state save/restore, and none
of the available kernel drivers do it on its behalf.
Since intelfb and the rest of the Intel drivers are fairly incompatible,
this patch makes the DRM bind to the PCI device so it can register real
suspend/resume handlers. Those handlers take care of saving and
restoring enough state for X to come back reliably on at least one of my
problematic test machines, but text mode still has trouble (still
debugging VGA state save/restore, including trying to save/restore
actual VRAM contents for possible hibernate support).
How does this approach look? Is a new DRM driver flag a good thing for
similar situations with other drivers? Thoughts?
Thanks,
Jesse
diff --git a/linux-core/Kconfig b/linux-core/Kconfig
index 2d02c76..5e73fc7 100644
--- a/linux-core/Kconfig
+++ b/linux-core/Kconfig
@@ -50,7 +50,7 @@ config DRM_I810
choice
prompt Intel 830M, 845G, 852GM, 855GM, 865G
- depends on DRM AGP AGP_INTEL
+ depends on DRM AGP AGP_INTEL !FB_INTEL
optional
config DRM_I915
@@ -60,6 +60,9 @@ config DRM_I915
852GM, 855GM, 865G, 915G, 915GM, 945G, 945GM and 965G integrated
graphics. If M is selected, the module will be called i915.
AGP support is required for this driver to work.
+
+ Note that this driver is incompatible with the Intel framebuffer
+ driver.
endchoice
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index f8ca3f4..36ce266 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -106,6 +106,7 @@ struct drm_file;
#define DRIVER_DMA_QUEUE 0x200
#define DRIVER_FB_DMA 0x400
#define DRIVER_IRQ_VBL20x800
+#define DRIVER_BIND_PCI0x1000
/[EMAIL PROTECTED]/
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
index a09fa96..4d11707 100644
--- a/linux-core/drm_drv.c
+++ b/linux-core/drm_drv.c
@@ -340,9 +340,10 @@ int drm_init(struct drm_driver *driver,
}
}
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver-driver_features DRIVER_BIND_PCI)) {
+ printk(KERN_ERR DRM binding to PCI device\n);
return pci_register_driver(driver-pci_driver);
- else {
+ } else {
for (i = 0; pciidlist[i].vendor != 0; i++) {
pid = pciidlist[i];
diff --git a/linux-core/i915_drv.c b/linux-core/i915_drv.c
index e337e1d..1f99944 100644
--- a/linux-core/i915_drv.c
+++ b/linux-core/i915_drv.c
@@ -69,6 +69,621 @@ static struct drm_bo_driver i915_bo_driver = {
};
#endif
+enum pipe {
+PIPE_A = 0,
+PIPE_B,
+};
+
+static bool i915_pipe_enabled(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ if (pipe == PIPE_A)
+ return (I915_READ(DPLL_A) DPLL_VCO_ENABLE);
+ else
+ return (I915_READ(DPLL_B) DPLL_VCO_ENABLE);
+}
+
+static void i915_save_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array = dev_priv-savePaletteA;
+ else
+ array = dev_priv-savePaletteB;
+
+ for(i = 0; i 256; i++)
+ array[i] = I915_READ(reg + (i 2));
+}
+
+static void i915_restore_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array = dev_priv-savePaletteA;
+ else
+ array = dev_priv-savePaletteB;
+
+ for(i = 0; i 256; i++)
+ I915_WRITE(reg + (i 2), array[i]);
+}
+
+static u8 i915_read_indexed(u16 index_port, u16 data_port, u8 reg)
+{
+ outb(reg, index_port);
+ return inb(data_port);
+}
+
+static void i915_write_indexed(u16 index_port, u16 data_port, u8 reg, u8 val)
+{
+ outb(reg, index_port);
+ outb(val, data_port);
+}
+
+static void i915_save_vga(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ unsigned long mem_mode, mem_offset = 0, mem_size = 0;
+ u8 *mapped_offset;
+ int i;
+ u16 cr_index, cr_data, st01;
+ u8 msr;
+
+ /* MSR bits */
+ msr = dev_priv-saveMSR = inb(VGA_MSR_READ);
+ if (dev_priv-saveMSR VGA_MSR_CGA_MODE) {
+ cr_index = VGA_CR_INDEX_CGA;
+ cr_data = VGA_CR_DATA_CGA;
+
Re: [patch] post superioctl inteface removal.
There is also the following (i don't think it was mentioned before in this thread): card with 8Mo of ram (who the hell have such hw ? :)) I've got 40 of them :( All of our desktops have integrated Intel ( i845g ) chips, and the BIOS has the option of stealing either 1MB or 8MB of system ram. It's hard to know whether to laugh or cry ... they don't count, the driver dynamically allocate main RAM into the aperture in this case :-) the initial BIOS allocation is just for text mode to get up and going, and VGA type things. Dave. - This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now http://get.splunk.com/ -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
[Bug 12855] glean abort with X Error of failed request: BadDrawable
http://bugs.freedesktop.org/show_bug.cgi?id=12855 --- Comment #2 from [EMAIL PROTECTED] 2007-10-18 19:16 PST --- Created an attachment (id=12115) -- (http://bugs.freedesktop.org/attachment.cgi?id=12115action=view) xorg log -- Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee. - This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now http://get.splunk.com/ -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
[Bug 12855] New: glean abort with X Error of failed request: BadDrawable
http://bugs.freedesktop.org/show_bug.cgi?id=12855 Summary: glean abort with X Error of failed request: BadDrawable Product: Mesa Version: CVS Platform: Other OS/Version: Linux (All) Status: NEW Severity: normal Priority: medium Component: Drivers/DRI/i915 AssignedTo: dri-devel@lists.sourceforge.net ReportedBy: [EMAIL PROTECTED] System Environment: -- --Platform: FC6 --Xf86_video_intel:79636b8c776ae024518103c9fa137c8498c21d48 --Mesa:a663e846b63c33685a9e88c8904f5e3a225f6e85 --DRM:2c5c18fbd394f419a9cf650720a1187440c643cd --Kernel:2.6.22 Bug detailed description: -- glean abort with following error: X Error of failed request: BadDrawable (invalid Pixmap or Window parameter) Major opcode of failed request: 145 (XFree86-DRI) Minor opcode of failed request: 8 () Resource id in failed request: 0x42 Serial number of failed request: 56 Current serial number in output stream: 64 Reproduce steps: start X ./glean -r resultlog Current result: api2: NOTE rgba8, db, z24, s8, win+pmap, id 37 Test skipped/non-applicable api2: NOTE rgba8, db, z24, s8, accrgba16, win+pmap, slow, id 41 Test skipped/non-applicable X Error of failed request: BadDrawable (invalid Pixmap or Window parameter) Major opcode of failed request: 145 (XFree86-DRI) Minor opcode of failed request: 8 () Resource id in failed request: 0x42 Serial number of failed request: 56 Current serial number in output stream: 64 Expected result: glean should run completely without error -- Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee. - This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now http://get.splunk.com/ -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [patch] post superioctl inteface removal.
Kristian Høgsberg wrote: On 10/18/07, Keith Whitwell [EMAIL PROTECTED] wrote: ... Doesn't Kristian changes to DRI interface (DRI2) already allow to clients to not care about front buffer. I mean if they all got private back buffer then they render into it. But i might have misunderstood this. Yes, of course. I'm not sure how watertight the isolation of the frontbuffer is in DRI2, but if it's complete and if the rule is that ttm supports only DRI2 clients, this concern would seem to be addressed. What I'm planning to do is to keep the front buffer bo handle in the kernel, associateed with the drm_drawable_t. The swap buffer ioctl will reference a drm_drawable_t as the front buffer destination and thus it will always be up to date and userspace won't hold any references to front buffers. The kernel holds a reference through each of the drm_drawable_t's that are non-redirected child-windows of the front buffer, and once the X server has re-attached the new front buffer bo to each drm_drawable_t, there should be no references to the front buffer. This happens in the SetWindowPixmap hook. It sounds like we want to free the old front buffer before we allocate the new one, in which case we'll have to call the SetWindowPixmap hook with a null pixmap or something to drop the kernel side references before allocating the new buffer. However, I was hoping we could avoid this, and allocate a new buffer while still scanning out from the old one, copy the contents, change the scan out address and then free the old one. This avoids flicker, and as for the fragmentation concern; can't we just alternate between allocating from different ends of the aperture? Or alternatively, do a two-step operation: allocate a temporary front buffer far enough into the aperture that we free the old buffer and then allocate the final new frontbuffer at offset 0? We should avoid turning off the crtcs here if at all possible. cheers, Kristian There is also the following (i don't think it was mentioned before in this thread): card with 8Mo of ram (who the hell have such hw ? :)) a scanout buffer which use 4Mo and user resizing to buffer which need 5Mo obviously we can't allocate it until we free the previous one... Maybe we can accept some kind of allocate over style or simply a resize function. Cheers, Jerome Glisse - This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now http://get.splunk.com/ -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [patch] post superioctl inteface removal.
On Fri, 2007-10-19 at 03:04 +0200, Jerome Glisse wrote: There is also the following (i don't think it was mentioned before in this thread): card with 8Mo of ram (who the hell have such hw ? :)) I've got 40 of them :( All of our desktops have integrated Intel ( i845g ) chips, and the BIOS has the option of stealing either 1MB or 8MB of system ram. It's hard to know whether to laugh or cry ... Dan - This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now http://get.splunk.com/ -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
[Bug 12855] glean abort with X Error of failed request: BadDrawable
http://bugs.freedesktop.org/show_bug.cgi?id=12855 --- Comment #1 from [EMAIL PROTECTED] 2007-10-18 19:15 PST --- Created an attachment (id=12114) -- (http://bugs.freedesktop.org/attachment.cgi?id=12114action=view) xorg conf -- Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee. - This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now http://get.splunk.com/ -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
