On Tue, Nov 3, 2026 at 3:28 AM Konrad Dybcio <[email protected]> wrote: > > On 11/2/26 8:00 PM, Rob Clark wrote: > > We need to move the gpu lock around a bit for the next patch. Use > > scoped_guard/etc so we don't have to make the error path handling more > > complicated. > > > > Signed-off-by: Rob Clark <[email protected]> > > --- > > drivers/gpu/drm/msm/msm_debugfs.c | 31 ++++++++++++------------------- > > 1 file changed, 12 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/msm_debugfs.c > > b/drivers/gpu/drm/msm/msm_debugfs.c > > index f12701e286ec..aa59b28a3392 100644 > > --- a/drivers/gpu/drm/msm/msm_debugfs.c > > +++ b/drivers/gpu/drm/msm/msm_debugfs.c > > @@ -70,43 +70,36 @@ static int msm_gpu_open(struct inode *inode, struct > > file *file) > > struct drm_device *dev = inode->i_private; > > struct msm_drm_private *priv = dev->dev_private; > > struct msm_gpu *gpu = priv->gpu; > > - struct msm_gpu_show_priv *show_priv; > > int ret; > > > > if (!gpu || !gpu->funcs->gpu_state_get) > > return -ENODEV; > > > > - show_priv = kmalloc_obj(*show_priv); > > + struct msm_gpu_show_priv *show_priv __free(kfree) > > + = kmalloc_obj(*show_priv); > > if (!show_priv) > > return -ENOMEM; > > > > - ret = mutex_lock_interruptible(&gpu->lock); > > - if (ret) > > - goto free_priv; > > - > > - pm_runtime_get_sync(&gpu->pdev->dev); > > - msm_gpu_hw_init(gpu); > > - show_priv->state = gpu->funcs->gpu_state_get(gpu); > > - pm_runtime_put_sync(&gpu->pdev->dev); > > - > > - mutex_unlock(&gpu->lock); > > + scoped_guard (pm_runtime_active, &gpu->pdev->dev) { > > + scoped_cond_guard (mutex_intr, return -EINTR, &gpu->lock) { > > + msm_gpu_hw_init(gpu); > > + show_priv->state = gpu->funcs->gpu_state_get(gpu); > > + } > > My clanker tells me we can't grab the PM reference before a mutex, > as the recovery mechanism depends on the rpm count being appropriate > to let it drop it (and kill gx this way)
Yeah, but we also can't do it in the other order, or more specifically disabling ifpc needs to come before gpu lock or it can deadlock with recovery.. but ifpc disable needs to be after runpm get. Re-working that ordering was the whole of introducing cleanup.h, to avoid the error path with mutex_lock_interruptible() after taking runpm I guess if you are hammering on debugfs while crashing the gpu, maybe the first recovery fails, and then a subsequent one eventually succeeds. BR, -R
