On Fri, Oct 14, 2011 at 10:14:54AM +0200, Michel Dänzer wrote:
> [ Dropping stable@xxxxxxxxxx from CC, it'll get picked up for stable
> once it hits mainline ]
>
> On Don, 2011-10-13 at 16:38 -0400, j.glisse@xxxxxxxxx wrote:
> > From: Jerome Glisse <jglisse@xxxxxxxxxx>
> >
> > After GPU lockup VRAM gart table is unpinned and thus its pointer
> > becomes unvalid. This patch move the unpin code to a common helper
> > function and set pointer to NULL so that page update code can check
> > if it should update GPU page table or not. That way bo still bound
> > to GART can be unbound (pci_unmap_page for all there page) properly
> > while there is no need to update the GPU page table.
> >
> > Signed-off-by: Jerome Glisse <jglisse@xxxxxxxxxx>
>
> [...]
>
> > rdev->gart.pages_addr[p] = rdev->dummy_page.addr;
> > page_base = rdev->gart.pages_addr[p];
> > for (j = 0; j < (PAGE_SIZE / RADEON_GPU_PAGE_SIZE); j++, t++) {
> > - radeon_gart_set_page(rdev, t, page_base);
> > + if (rdev->gart.ptr) {
> > + radeon_gart_set_page(rdev, t, page_base);
> > + }
> > page_base += RADEON_GPU_PAGE_SIZE;
> > }
> > }
> > @@ -201,7 +211,9 @@ int radeon_gart_bind(struct radeon_device *rdev, unsigned offset,
> > rdev->gart.pages[p] = pagelist[i];
> > page_base = rdev->gart.pages_addr[p];
> > for (j = 0; j < (PAGE_SIZE / RADEON_GPU_PAGE_SIZE); j++, t++) {
> > - radeon_gart_set_page(rdev, t, page_base);
> > + if (rdev->gart.ptr) {
> > + radeon_gart_set_page(rdev, t, page_base);
> > + }
> > page_base += RADEON_GPU_PAGE_SIZE;
> > }
> > }
>
> Looks like these two loops could be completely guarded by if (rdev->gart.ptr).
Will respin with that
>
> > @@ -218,7 +230,9 @@ void radeon_gart_restore(struct radeon_device *rdev)
> > for (i = 0, t = 0; i < rdev->gart.num_cpu_pages; i++) {
> > page_base = rdev->gart.pages_addr[i];
> > for (j = 0; j < (PAGE_SIZE / RADEON_GPU_PAGE_SIZE); j++, t++) {
> > - radeon_gart_set_page(rdev, t, page_base);
> > + if (rdev->gart.ptr) {
> > + radeon_gart_set_page(rdev, t, page_base);
> > + }
> > page_base += RADEON_GPU_PAGE_SIZE;
> > }
> > }
>
> I think this should rather BUG_ON(!rdev->gart.ptr).
This should only happen after GPU lockup and GPU lockup would already
print loud message so i don't think it's usefull to add another message
that would confuse user more.
Cheers,
Jerome
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
