Nouveau - Nov 2024 - [PATCH] nouveau: fw: sync dma after setup is called.

From: Dave Airlie <airlied at redhat.com>

When this code moved to non-coherent allocator the sync was put too
early for some firmwares which called the setup function, move the
sync down after the setup function.

Reported-by: Diogo Ivo <diogo.ivo at tecnico.ulisboa.pt>
Tested-by: Diogo Ivo <diogo.ivo at tecnico.ulisboa.pt>
Fixes: 9b340aeb26d5 ("nouveau/firmware: use dma non-coherent
allocator")
Cc: stable at vger.kernel.org
Signed-off-by: Dave Airlie <airlied at redhat.com>
---
 drivers/gpu/drm/nouveau/nvkm/falcon/fw.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
b/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
index a1c8545f1249..cac6d64ab67d 100644
--- a/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
+++ b/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
@@ -89,11 +89,6 @@ nvkm_falcon_fw_boot(struct nvkm_falcon_fw *fw, struct
nvkm_subdev *user,
 		nvkm_falcon_fw_dtor_sigs(fw);
 	}
 
-	/* after last write to the img, sync dma mappings */
-	dma_sync_single_for_device(fw->fw.device->dev,
-				   fw->fw.phys,
-				   sg_dma_len(&fw->fw.mem.sgl),
-				   DMA_TO_DEVICE);
 
 	FLCNFW_DBG(fw, "resetting");
 	fw->func->reset(fw);
@@ -105,6 +100,12 @@ nvkm_falcon_fw_boot(struct nvkm_falcon_fw *fw, struct
nvkm_subdev *user,
 			goto done;
 	}
 
+	/* after last write to the img, sync dma mappings */
+	dma_sync_single_for_device(fw->fw.device->dev,
+				   fw->fw.phys,
+				   sg_dma_len(&fw->fw.mem.sgl),
+				   DMA_TO_DEVICE);
+
 	ret = fw->func->load(fw);
 	if (ret)
 		goto done;
-- 
2.47.0

On 14/11/24 10:46, Dave Airlie wrote:
> From: Dave Airlie <airlied at redhat.com>
>
> When this code moved to non-coherent allocator the sync was put too
> early for some firmwares which called the setup function, move the
> sync down after the setup function.
>
> Reported-by: Diogo Ivo <diogo.ivo at tecnico.ulisboa.pt>
> Tested-by: Diogo Ivo <diogo.ivo at tecnico.ulisboa.pt>
> Fixes: 9b340aeb26d5 ("nouveau/firmware: use dma non-coherent
allocator")
> Cc: stable at vger.kernel.org
> Signed-off-by: Dave Airlie <airlied at redhat.com>
Reviewed-by: Ben Skeggs <bskeggs at nvidia.com>
> ---
>   drivers/gpu/drm/nouveau/nvkm/falcon/fw.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
b/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
> index a1c8545f1249..cac6d64ab67d 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
> @@ -89,11 +89,6 @@ nvkm_falcon_fw_boot(struct nvkm_falcon_fw *fw, struct
nvkm_subdev *user,
>   		nvkm_falcon_fw_dtor_sigs(fw);
>   	}
>   
> -	/* after last write to the img, sync dma mappings */
> -	dma_sync_single_for_device(fw->fw.device->dev,
> -				   fw->fw.phys,
> -				   sg_dma_len(&fw->fw.mem.sgl),
> -				   DMA_TO_DEVICE);
>   
>   	FLCNFW_DBG(fw, "resetting");
>   	fw->func->reset(fw);
> @@ -105,6 +100,12 @@ nvkm_falcon_fw_boot(struct nvkm_falcon_fw *fw, struct
nvkm_subdev *user,
>   			goto done;
>   	}
>   
> +	/* after last write to the img, sync dma mappings */
> +	dma_sync_single_for_device(fw->fw.device->dev,
> +				   fw->fw.phys,
> +				   sg_dma_len(&fw->fw.mem.sgl),
> +				   DMA_TO_DEVICE);
> +
>   	ret = fw->func->load(fw);
>   	if (ret)
>   		goto done;

On 11/14/24 1:46 AM, Dave Airlie wrote:
> From: Dave Airlie <airlied at redhat.com>
> 
> When this code moved to non-coherent allocator the sync was put too
> early for some firmwares which called the setup function, move the
> sync down after the setup function.
> 
> Reported-by: Diogo Ivo <diogo.ivo at tecnico.ulisboa.pt>
Do you have a link of where this issue has been reported?
> Tested-by: Diogo Ivo <diogo.ivo at tecnico.ulisboa.pt>
> Fixes: 9b340aeb26d5 ("nouveau/firmware: use dma non-coherent
allocator")
> Cc: stable at vger.kernel.org
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>   drivers/gpu/drm/nouveau/nvkm/falcon/fw.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
b/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
> index a1c8545f1249..cac6d64ab67d 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
> @@ -89,11 +89,6 @@ nvkm_falcon_fw_boot(struct nvkm_falcon_fw *fw, struct
nvkm_subdev *user,
>   		nvkm_falcon_fw_dtor_sigs(fw);
>   	}
>   
> -	/* after last write to the img, sync dma mappings */
> -	dma_sync_single_for_device(fw->fw.device->dev,
> -				   fw->fw.phys,
> -				   sg_dma_len(&fw->fw.mem.sgl),
> -				   DMA_TO_DEVICE);
>   
>   	FLCNFW_DBG(fw, "resetting");
>   	fw->func->reset(fw);
> @@ -105,6 +100,12 @@ nvkm_falcon_fw_boot(struct nvkm_falcon_fw *fw, struct
nvkm_subdev *user,
>   			goto done;
>   	}
>   
> +	/* after last write to the img, sync dma mappings */
> +	dma_sync_single_for_device(fw->fw.device->dev,
> +				   fw->fw.phys,
> +				   sg_dma_len(&fw->fw.mem.sgl),
> +				   DMA_TO_DEVICE);
> +
>   	ret = fw->func->load(fw);
>   	if (ret)
>   		goto done;