Re: [PATCH 3/5] drm/msm/adreno: Load the firmware before bringing up the hardware

[Sharat Masetty <smasetty-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

On 8/6/2018 11:03 PM, Jordan Crouse wrote:
> Failure to load firmware is the primary reason to fail adreno_load_gpu().
> Try to load it first before going into the hardware initialization code and
> unwinding it. This is important for a6xx because the GMU gets loaded from
> the runtime power code and it is more costly to fail in that path because
> of missing firmware.
> 
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
>   drivers/gpu/drm/msm/adreno/adreno_device.c | 23 +++++++++++++++++++++-
>   drivers/gpu/drm/msm/adreno/adreno_gpu.c    |  2 +-
>   drivers/gpu/drm/msm/adreno/adreno_gpu.h    |  2 +-
>   3 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index 44813624a286..37746f1d54cf 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -155,6 +155,7 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev)
>   	struct msm_drm_private *priv = dev->dev_private;
>   	struct platform_device *pdev = priv->gpu_pdev;
>   	struct msm_gpu *gpu = NULL;
> +	struct adreno_gpu *adreno_gpu;
>   	int ret;
>   
>   	if (pdev)
> @@ -165,7 +166,27 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev)
>   		return NULL;
>   	}
>   
> -	pm_runtime_get_sync(&pdev->dev);
> +	adreno_gpu = to_adreno_gpu(gpu);
> +
> +	/*
> +	 * The number one reason for HW init to fail is if the firmware isn't
> +	 * loaded yet. Try that first and don't bother continuing on
> +	 * otherwise
> +	 */
> +
> +	ret = adreno_load_fw(adreno_gpu);
> +	if (ret)
> +		return NULL;
This is a good idea, but you should remove the call to adreno_load_fw() 
from adreno_hw_init() as it should no longer be needed with this change.

> +
> +	/* Make sure pm runtime is active and reset any previous errors */
> +	pm_runtime_set_active(&pdev->dev);
> +
> +	ret = pm_runtime_get_sync(&pdev->dev);
> +	if (ret < 0) {
> +		dev_err(dev->dev, "Couldn't power up the GPU: %d\n", ret);
> +		return NULL;
> +	}
> +
>   	mutex_lock(&dev->struct_mutex);
>   	ret = msm_gpu_hw_init(gpu);
>   	mutex_unlock(&dev->struct_mutex);
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 65c0ae7d8ad1..da1363a0c54d 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -149,7 +149,7 @@ adreno_request_fw(struct adreno_gpu *adreno_gpu, const char *fwname)
>   	return fw;
>   }
>   
> -static int adreno_load_fw(struct adreno_gpu *adreno_gpu)
> +int adreno_load_fw(struct adreno_gpu *adreno_gpu)
>   {
>   	int i;
>   
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index 4406776597fd..d391ff377612 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -228,7 +228,7 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>   		struct adreno_gpu *gpu, const struct adreno_gpu_funcs *funcs,
>   		int nr_rings);
>   void adreno_gpu_cleanup(struct adreno_gpu *gpu);
> -
> +int adreno_load_fw(struct adreno_gpu *adreno_gpu);
>   
>   void adreno_gpu_state_destroy(struct msm_gpu_state *state);
>   
> 

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno