* [bug report] drm/panthor: Call panthor_gpu_coherency_init() after PM resume()
@ 2025-04-12 14:39 Dan Carpenter
2025-04-14 7:34 ` Boris Brezillon
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2025-04-12 14:39 UTC (permalink / raw)
To: Boris Brezillon; +Cc: dri-devel
Hello Boris Brezillon,
Commit 7d5a3b22f5b5 ("drm/panthor: Call panthor_gpu_coherency_init()
after PM resume()") from Apr 4, 2025 (linux-next), leads to the
following Smatch static checker warning:
drivers/gpu/drm/panthor/panthor_device.c:248 panthor_device_init()
warn: missing unwind goto?
drivers/gpu/drm/panthor/panthor_device.c
167 int panthor_device_init(struct panthor_device *ptdev)
168 {
169 u32 *dummy_page_virt;
170 struct resource *res;
171 struct page *p;
172 int ret;
173
174 init_completion(&ptdev->unplug.done);
175 ret = drmm_mutex_init(&ptdev->base, &ptdev->unplug.lock);
176 if (ret)
177 return ret;
178
179 ret = drmm_mutex_init(&ptdev->base, &ptdev->pm.mmio_lock);
180 if (ret)
181 return ret;
182
183 atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_SUSPENDED);
184 p = alloc_page(GFP_KERNEL | __GFP_ZERO);
185 if (!p)
186 return -ENOMEM;
187
188 ptdev->pm.dummy_latest_flush = p;
189 dummy_page_virt = page_address(p);
190 ret = drmm_add_action_or_reset(&ptdev->base, panthor_device_free_page,
191 ptdev->pm.dummy_latest_flush);
192 if (ret)
193 return ret;
194
195 /*
196 * Set the dummy page holding the latest flush to 1. This will cause the
197 * flush to avoided as we know it isn't necessary if the submission
198 * happens while the dummy page is mapped. Zero cannot be used because
199 * that means 'always flush'.
200 */
201 *dummy_page_virt = 1;
202
203 INIT_WORK(&ptdev->reset.work, panthor_device_reset_work);
204 ptdev->reset.wq = alloc_ordered_workqueue("panthor-reset-wq", 0);
205 if (!ptdev->reset.wq)
206 return -ENOMEM;
207
208 ret = drmm_add_action_or_reset(&ptdev->base, panthor_device_reset_cleanup, NULL);
209 if (ret)
210 return ret;
211
212 ret = panthor_clk_init(ptdev);
213 if (ret)
214 return ret;
215
216 ret = panthor_devfreq_init(ptdev);
217 if (ret)
218 return ret;
219
220 ptdev->iomem = devm_platform_get_and_ioremap_resource(to_platform_device(ptdev->base.dev),
221 0, &res);
222 if (IS_ERR(ptdev->iomem))
223 return PTR_ERR(ptdev->iomem);
224
225 ptdev->phys_addr = res->start;
226
227 ret = devm_pm_runtime_enable(ptdev->base.dev);
228 if (ret)
229 return ret;
230
231 ret = pm_runtime_resume_and_get(ptdev->base.dev);
232 if (ret)
233 return ret;
234
235 /* If PM is disabled, we need to call panthor_device_resume() manually. */
236 if (!IS_ENABLED(CONFIG_PM)) {
237 ret = panthor_device_resume(ptdev->base.dev);
238 if (ret)
239 return ret;
240 }
241
242 ret = panthor_gpu_init(ptdev);
243 if (ret)
244 goto err_rpm_put;
245
246 ret = panthor_gpu_coherency_init(ptdev);
247 if (ret)
--> 248 return ret;
^^^^^^^^^^^
Missing cleanup?
249
250 ret = panthor_mmu_init(ptdev);
251 if (ret)
252 goto err_unplug_gpu;
253
254 ret = panthor_fw_init(ptdev);
255 if (ret)
256 goto err_unplug_mmu;
257
258 ret = panthor_sched_init(ptdev);
259 if (ret)
260 goto err_unplug_fw;
261
262 /* ~3 frames */
263 pm_runtime_set_autosuspend_delay(ptdev->base.dev, 50);
264 pm_runtime_use_autosuspend(ptdev->base.dev);
265
266 ret = drm_dev_register(&ptdev->base, 0);
267 if (ret)
268 goto err_disable_autosuspend;
269
270 pm_runtime_put_autosuspend(ptdev->base.dev);
271 return 0;
272
273 err_disable_autosuspend:
274 pm_runtime_dont_use_autosuspend(ptdev->base.dev);
275 panthor_sched_unplug(ptdev);
276
277 err_unplug_fw:
278 panthor_fw_unplug(ptdev);
279
280 err_unplug_mmu:
281 panthor_mmu_unplug(ptdev);
282
283 err_unplug_gpu:
284 panthor_gpu_unplug(ptdev);
285
286 err_rpm_put:
287 pm_runtime_put_sync_suspend(ptdev->base.dev);
288 return ret;
289 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [bug report] drm/panthor: Call panthor_gpu_coherency_init() after PM resume()
2025-04-12 14:39 [bug report] drm/panthor: Call panthor_gpu_coherency_init() after PM resume() Dan Carpenter
@ 2025-04-14 7:34 ` Boris Brezillon
2025-04-14 7:51 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Boris Brezillon @ 2025-04-14 7:34 UTC (permalink / raw)
To: Dan Carpenter; +Cc: dri-devel
On Sat, 12 Apr 2025 17:39:58 +0300
Dan Carpenter <dan.carpenter@linaro.org> wrote:
> Hello Boris Brezillon,
>
> Commit 7d5a3b22f5b5 ("drm/panthor: Call panthor_gpu_coherency_init()
> after PM resume()") from Apr 4, 2025 (linux-next), leads to the
> following Smatch static checker warning:
>
> drivers/gpu/drm/panthor/panthor_device.c:248 panthor_device_init()
> warn: missing unwind goto?
>
> drivers/gpu/drm/panthor/panthor_device.c
> 167 int panthor_device_init(struct panthor_device *ptdev)
> 168 {
> 169 u32 *dummy_page_virt;
> 170 struct resource *res;
> 171 struct page *p;
> 172 int ret;
> 173
> 174 init_completion(&ptdev->unplug.done);
> 175 ret = drmm_mutex_init(&ptdev->base, &ptdev->unplug.lock);
> 176 if (ret)
> 177 return ret;
> 178
> 179 ret = drmm_mutex_init(&ptdev->base, &ptdev->pm.mmio_lock);
> 180 if (ret)
> 181 return ret;
> 182
> 183 atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_SUSPENDED);
> 184 p = alloc_page(GFP_KERNEL | __GFP_ZERO);
> 185 if (!p)
> 186 return -ENOMEM;
> 187
> 188 ptdev->pm.dummy_latest_flush = p;
> 189 dummy_page_virt = page_address(p);
> 190 ret = drmm_add_action_or_reset(&ptdev->base, panthor_device_free_page,
> 191 ptdev->pm.dummy_latest_flush);
> 192 if (ret)
> 193 return ret;
> 194
> 195 /*
> 196 * Set the dummy page holding the latest flush to 1. This will cause the
> 197 * flush to avoided as we know it isn't necessary if the submission
> 198 * happens while the dummy page is mapped. Zero cannot be used because
> 199 * that means 'always flush'.
> 200 */
> 201 *dummy_page_virt = 1;
> 202
> 203 INIT_WORK(&ptdev->reset.work, panthor_device_reset_work);
> 204 ptdev->reset.wq = alloc_ordered_workqueue("panthor-reset-wq", 0);
> 205 if (!ptdev->reset.wq)
> 206 return -ENOMEM;
> 207
> 208 ret = drmm_add_action_or_reset(&ptdev->base, panthor_device_reset_cleanup, NULL);
> 209 if (ret)
> 210 return ret;
> 211
> 212 ret = panthor_clk_init(ptdev);
> 213 if (ret)
> 214 return ret;
> 215
> 216 ret = panthor_devfreq_init(ptdev);
> 217 if (ret)
> 218 return ret;
> 219
> 220 ptdev->iomem = devm_platform_get_and_ioremap_resource(to_platform_device(ptdev->base.dev),
> 221 0, &res);
> 222 if (IS_ERR(ptdev->iomem))
> 223 return PTR_ERR(ptdev->iomem);
> 224
> 225 ptdev->phys_addr = res->start;
> 226
> 227 ret = devm_pm_runtime_enable(ptdev->base.dev);
> 228 if (ret)
> 229 return ret;
> 230
> 231 ret = pm_runtime_resume_and_get(ptdev->base.dev);
> 232 if (ret)
> 233 return ret;
> 234
> 235 /* If PM is disabled, we need to call panthor_device_resume() manually. */
> 236 if (!IS_ENABLED(CONFIG_PM)) {
> 237 ret = panthor_device_resume(ptdev->base.dev);
> 238 if (ret)
> 239 return ret;
> 240 }
> 241
> 242 ret = panthor_gpu_init(ptdev);
> 243 if (ret)
> 244 goto err_rpm_put;
> 245
> 246 ret = panthor_gpu_coherency_init(ptdev);
> 247 if (ret)
> --> 248 return ret;
> ^^^^^^^^^^^
> Missing cleanup?
Thanks for the report, it should definitely be
goto err_unplug_gpu;
Do you plan to send a patch, or should I do it?
>
> 249
> 250 ret = panthor_mmu_init(ptdev);
> 251 if (ret)
> 252 goto err_unplug_gpu;
> 253
> 254 ret = panthor_fw_init(ptdev);
> 255 if (ret)
> 256 goto err_unplug_mmu;
> 257
> 258 ret = panthor_sched_init(ptdev);
> 259 if (ret)
> 260 goto err_unplug_fw;
> 261
> 262 /* ~3 frames */
> 263 pm_runtime_set_autosuspend_delay(ptdev->base.dev, 50);
> 264 pm_runtime_use_autosuspend(ptdev->base.dev);
> 265
> 266 ret = drm_dev_register(&ptdev->base, 0);
> 267 if (ret)
> 268 goto err_disable_autosuspend;
> 269
> 270 pm_runtime_put_autosuspend(ptdev->base.dev);
> 271 return 0;
> 272
> 273 err_disable_autosuspend:
> 274 pm_runtime_dont_use_autosuspend(ptdev->base.dev);
> 275 panthor_sched_unplug(ptdev);
> 276
> 277 err_unplug_fw:
> 278 panthor_fw_unplug(ptdev);
> 279
> 280 err_unplug_mmu:
> 281 panthor_mmu_unplug(ptdev);
> 282
> 283 err_unplug_gpu:
> 284 panthor_gpu_unplug(ptdev);
> 285
> 286 err_rpm_put:
> 287 pm_runtime_put_sync_suspend(ptdev->base.dev);
> 288 return ret;
> 289 }
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [bug report] drm/panthor: Call panthor_gpu_coherency_init() after PM resume()
2025-04-14 7:34 ` Boris Brezillon
@ 2025-04-14 7:51 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2025-04-14 7:51 UTC (permalink / raw)
To: Boris Brezillon; +Cc: dri-devel
On Mon, Apr 14, 2025 at 09:34:48AM +0200, Boris Brezillon wrote:
> > 235 /* If PM is disabled, we need to call panthor_device_resume() manually. */
> > 236 if (!IS_ENABLED(CONFIG_PM)) {
> > 237 ret = panthor_device_resume(ptdev->base.dev);
> > 238 if (ret)
> > 239 return ret;
> > 240 }
> > 241
> > 242 ret = panthor_gpu_init(ptdev);
> > 243 if (ret)
> > 244 goto err_rpm_put;
> > 245
> > 246 ret = panthor_gpu_coherency_init(ptdev);
> > 247 if (ret)
> > --> 248 return ret;
> > ^^^^^^^^^^^
> > Missing cleanup?
>
> Thanks for the report, it should definitely be
>
> goto err_unplug_gpu;
>
> Do you plan to send a patch, or should I do it?
>
I'm on vacation.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-04-14 7:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-12 14:39 [bug report] drm/panthor: Call panthor_gpu_coherency_init() after PM resume() Dan Carpenter
2025-04-14 7:34 ` Boris Brezillon
2025-04-14 7:51 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.