Re: [Freedreno] [PATCH 3/5] drm/msm/adreno: Load the firmware before bringing up the hardware
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
---
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(>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(>dev);
+
+ ret = pm_runtime_get_sync(>dev);
+ if (ret < 0) {
+ dev_err(dev->dev, "Couldn't power up the GPU: %d\n", ret);
+ return NULL;
+ }
+
mutex_lock(>struct_mutex);
ret = msm_gpu_hw_init(gpu);
mutex_unlock(>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
[Freedreno] [PATCH 3/5] drm/msm/adreno: Load the firmware before bringing up the hardware
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
---
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(>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;
+
+ /* Make sure pm runtime is active and reset any previous errors */
+ pm_runtime_set_active(>dev);
+
+ ret = pm_runtime_get_sync(>dev);
+ if (ret < 0) {
+ dev_err(dev->dev, "Couldn't power up the GPU: %d\n", ret);
+ return NULL;
+ }
+
mutex_lock(>struct_mutex);
ret = msm_gpu_hw_init(gpu);
mutex_unlock(>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);
--
2.18.0
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 3/5] drm/msm/adreno: Load the firmware before bringing up the hardware
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.
[v2 - fix compile error caused by adreno_load_fw declared static in
adreno_gpu ]
Signed-off-by: Jordan Crouse
---
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(>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;
+
+ /* Make sure pm runtime is active and reset any previous errors */
+ pm_runtime_set_active(>dev);
+
+ ret = pm_runtime_get_sync(>dev);
+ if (ret < 0) {
+ dev_err(dev->dev, "Couldn't power up the GPU: %d\n", ret);
+ return NULL;
+ }
+
mutex_lock(>struct_mutex);
ret = msm_gpu_hw_init(gpu);
mutex_unlock(>struct_mutex);
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index bcbf9f2a29f9..ae693c28d66c 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -138,7 +138,7 @@ adreno_request_fw(struct adreno_gpu *adreno_gpu, const char
*fwname)
return ERR_PTR(-ENOENT);
}
-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 bc9ec27e9ed8..81397935d847 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -227,7 +227,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);
/* ringbuffer helpers (the parts that are adreno specific) */
--
2.17.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 3/5] drm/msm/adreno: Load the firmware before bringing up the hardware
Hi Jordan,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on robclark/msm-next]
[also build test ERROR on v4.17 next-20180608]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Jordan-Crouse/drm-msm-Add-support-for-Adreno-a6xx/20180612-022828
base: git://people.freedesktop.org/~robclark/linux msm-next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=arm64
Note: the
linux-review/Jordan-Crouse/drm-msm-Add-support-for-Adreno-a6xx/20180612-022828
HEAD 716a0bd2934f1b01e622ac7a6714789861824f8f builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
drivers/gpu/drm/msm/adreno/adreno_device.c: In function 'adreno_load_gpu':
>> drivers/gpu/drm/msm/adreno/adreno_device.c:177:8: error: implicit
>> declaration of function 'adreno_load_fw'; did you mean 'adreno_load_gpu'?
>> [-Werror=implicit-function-declaration]
ret = adreno_load_fw(adreno_gpu);
^~
adreno_load_gpu
cc1: some warnings being treated as errors
vim +177 drivers/gpu/drm/msm/adreno/adreno_device.c
152
153 struct msm_gpu *adreno_load_gpu(struct drm_device *dev)
154 {
155 struct msm_drm_private *priv = dev->dev_private;
156 struct platform_device *pdev = priv->gpu_pdev;
157 struct msm_gpu *gpu = NULL;
158 struct adreno_gpu *adreno_gpu;
159 int ret;
160
161 if (pdev)
162 gpu = platform_get_drvdata(pdev);
163
164 if (!gpu) {
165 dev_err_once(dev->dev, "no GPU device was found\n");
166 return NULL;
167 }
168
169 adreno_gpu = to_adreno_gpu(gpu);
170
171 /*
172 * The number one reason for HW init to fail is if the firmware
isn't
173 * loaded yet. Try that first and don't bother continuing on
174 * otherwise
175 */
176
> 177 ret = adreno_load_fw(adreno_gpu);
178 if (ret)
179 return NULL;
180
181 /* Make sure pm runtime is active and reset any previous errors
*/
182 pm_runtime_set_active(>dev);
183
184 ret = pm_runtime_get_sync(>dev);
185 if (ret < 0) {
186 dev_err(dev->dev, "Couldn't power up the GPU: %d\n",
ret);
187 return NULL;
188 }
189
190 mutex_lock(>struct_mutex);
191 ret = msm_gpu_hw_init(gpu);
192 mutex_unlock(>struct_mutex);
193 pm_runtime_put_autosuspend(>dev);
194 if (ret) {
195 dev_err(dev->dev, "gpu hw init failed: %d\n", ret);
196 return NULL;
197 }
198
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 3/5] drm/msm/adreno: Load the firmware before bringing up the hardware
Failure to load firwmare 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
---
drivers/gpu/drm/msm/adreno/adreno_device.c | 23 +-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 0c0eaad68065..d70e7d145dae 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(>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;
+
+ /* Make sure pm runtime is active and reset any previous errors */
+ pm_runtime_set_active(>dev);
+
+ ret = pm_runtime_get_sync(>dev);
+ if (ret < 0) {
+ dev_err(dev->dev, "Couldn't power up the GPU: %d\n", ret);
+ return NULL;
+ }
+
mutex_lock(>struct_mutex);
ret = msm_gpu_hw_init(gpu);
mutex_unlock(>struct_mutex);
--
2.17.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
