On Thu, 2010-04-01 at 16:54 +0200, Jerome Glisse wrote:
>
> From: Jerome Glisse <jgli...@redhat.com>
> Date: Thu, 1 Apr 2010 16:46:15 +0200
> Subject: [PATCH] agp: force use of scratch page
>
> It seems that some AGP hardware can induce activity on bus
> after reporting being idle with the memory (preteching maybe).
> As new infrastructure for GPU is dynamicly binding/unbinding
> memory this result in hardware possibly trying to access invalid
> memory to avoid this situation the patch for the use of the
> scratch page mecanism for all the AGP driver. It remove the
> needs_scratch_page flags and fix the insert_memory/remove_memory
> to deal properly with scratch_page.
>
> Been tested on intel & sis agp bridges.
>
> Signed-off-by: Jerome Glisse <jgli...@redhat.com>
Please don't attach patches but use git send-email or so for easier review.
> diff --git a/drivers/char/agp/backend.c b/drivers/char/agp/backend.c
> index c3ab46d..fed5e46 100644
> --- a/drivers/char/agp/backend.c
> +++ b/drivers/char/agp/backend.c
> @@ -136,35 +136,32 @@ static int agp_find_max(void)
> static int agp_backend_initialize(struct agp_bridge_data *bridge)
> {
> int size_value, rc, got_gatt=0, got_keylist=0;
> + struct page *page;
> + void *va;
>
> bridge->max_memory_agp = agp_find_max();
> bridge->version = &agp_current_version;
>
> - if (bridge->driver->needs_scratch_page) {
> - struct page *page = bridge->driver->agp_alloc_page(bridge);
> -
> - if (!page) {
> - dev_err(&bridge->dev->dev,
> + page = bridge->driver->agp_alloc_page(bridge);
> + if (!page) {
> + dev_err(&bridge->dev->dev,
> "can't get memory for scratch page\n");
> - return -ENOMEM;
> - }
> -
> - bridge->scratch_page_page = page;
> - if (bridge->driver->agp_map_page) {
> - if (bridge->driver->agp_map_page(page,
> -
> &bridge->scratch_page_dma)) {
> - dev_err(&bridge->dev->dev,
> + return -ENOMEM;
> + }
> + bridge->scratch_page_page = page;
> + if (bridge->driver->agp_map_page) {
> + if (bridge->driver->agp_map_page(page,
> + &bridge->scratch_page_dma)) {
> + dev_err(&bridge->dev->dev,
> "unable to dma-map scratch page\n");
> - rc = -ENOMEM;
> - goto err_out_nounmap;
> - }
> - } else {
> - bridge->scratch_page_dma = page_to_phys(page);
> + rc = -ENOMEM;
> + goto err_out_nounmap;
> }
> -
> - bridge->scratch_page = bridge->driver->mask_memory(bridge,
> - bridge->scratch_page_dma, 0);
> + } else {
> + bridge->scratch_page_dma = page_to_phys(page);
> }
> + bridge->scratch_page = bridge->driver->mask_memory(bridge,
> + bridge->scratch_page_dma, 0);
>
> size_value = bridge->driver->fetch_size();
> if (size_value == 0) {
> @@ -203,18 +200,15 @@ static int agp_backend_initialize(struct
> agp_bridge_data *bridge)
> return 0;
>
> err_out:
> - if (bridge->driver->needs_scratch_page &&
> - bridge->driver->agp_unmap_page) {
> + if (bridge->driver->agp_unmap_page) {
> bridge->driver->agp_unmap_page(bridge->scratch_page_page,
> bridge->scratch_page_dma);
> }
> err_out_nounmap:
> - if (bridge->driver->needs_scratch_page) {
> - void *va = page_address(bridge->scratch_page_page);
> + va = page_address(bridge->scratch_page_page);
> + bridge->driver->agp_destroy_page(va, AGP_PAGE_DESTROY_UNMAP);
> + bridge->driver->agp_destroy_page(va, AGP_PAGE_DESTROY_FREE);
>
> - bridge->driver->agp_destroy_page(va, AGP_PAGE_DESTROY_UNMAP);
> - bridge->driver->agp_destroy_page(va, AGP_PAGE_DESTROY_FREE);
> - }
> if (got_gatt)
> bridge->driver->free_gatt_table(bridge);
> if (got_keylist) {
> @@ -227,6 +221,8 @@ err_out_nounmap:
> /* cannot be __exit b/c as it could be called from __init code */
> static void agp_backend_cleanup(struct agp_bridge_data *bridge)
> {
> + void *va;
> +
> if (bridge->driver->cleanup)
> bridge->driver->cleanup();
> if (bridge->driver->free_gatt_table)
> @@ -235,9 +231,8 @@ static void agp_backend_cleanup(struct agp_bridge_data
> *bridge)
> vfree(bridge->key_list);
> bridge->key_list = NULL;
>
> - if (bridge->driver->agp_destroy_page &&
> - bridge->driver->needs_scratch_page) {
> - void *va = page_address(bridge->scratch_page_page);
> + if (bridge->driver->agp_destroy_page) {
> + va = page_address(bridge->scratch_page_page);
>
> if (bridge->driver->agp_unmap_page)
>
> bridge->driver->agp_unmap_page(bridge->scratch_page_page,
AFAICT this just removes the needs_scratch_page flag but doesn't bind
all table entries to the scratch page. Am I missing something?
> diff --git a/drivers/char/agp/uninorth-agp.c b/drivers/char/agp/uninorth-agp.c
> index d89da4a..d54d674 100644
> --- a/drivers/char/agp/uninorth-agp.c
> +++ b/drivers/char/agp/uninorth-agp.c
> @@ -171,7 +171,7 @@ static int uninorth_insert_memory(struct agp_memory *mem,
> off_t pg_start, int ty
>
> gp = (u32 *) &agp_bridge->gatt_table[pg_start];
> for (i = 0; i < mem->page_count; ++i) {
> - if (gp[i]) {
> + if (!PGE_EMPTY(agp_bridge, gp[i])) {
> dev_info(&agp_bridge->dev->dev,
> "uninorth_insert_memory: entry 0x%x occupied
> (%x)\n",
> i, gp[i]);
>
This won't work for pre-U3 bridges, as bound table entries are marked by
setting bit 0. (Maybe the driver just shouldn't use
agp_generic_mask_memory?)
--
Earthling Michel Dänzer | http://www.vmware.com
Libre software enthusiast | Debian, X and DRI developer
------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
