[PATCH 03/10] drm/amdgpu: handle runtime pm in fbcon

[Deucher, Alexander]

> -----Original Message-----
> From: Michel Dänzer [mailto:michel at daenzer.net]
> Sent: Thursday, September 01, 2016 9:47 PM
> To: Alex Deucher
> Cc: amd-gfx list; Deucher, Alexander
> Subject: Re: [PATCH 03/10] drm/amdgpu: handle runtime pm in fbcon
> 
> On 02/09/16 05:44 AM, Alex Deucher wrote:
> > On Wed, Aug 31, 2016 at 9:04 PM, Michel Dänzer <michel at daenzer.net>
> wrote:
> >> On 01/09/16 07:08 AM, Alex Deucher wrote:
> >>> Ported from nouveau.
> >>>
> >>> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> >>
> >> [...]
> >>
> >>> +static int
> >>> +amdgpufb_open(struct fb_info *info, int user)
> >>> +{
> >>> +     struct amdgpu_fbdev *rfbdev = info->par;
> >>> +     struct amdgpu_device *adev = rfbdev->adev;
> >>> +     int ret = pm_runtime_get_sync(adev->ddev->dev);
> >>> +     if (ret < 0 && ret != -EACCES)
> >>> +             return ret;
> >>> +     return 0;
> >>> +}
> >>> +
> >>> +static int
> >>> +amdgpufb_release(struct fb_info *info, int user)
> >>> +{
> >>> +     struct amdgpu_fbdev *rfbdev = info->par;
> >>> +     struct amdgpu_device *adev = rfbdev->adev;
> >>> +
> >>> +     pm_runtime_mark_last_busy(adev->ddev->dev);
> >>> +     pm_runtime_put_autosuspend(adev->ddev->dev);
> >>> +     return 0;
> >>> +}
> >>
> >> If pm_runtime_get_sync returns -EACCES, won't there be an imbalance
> with
> >> the pm_runtime_put_autosuspend call, which might result in the GPU
> >> powering off even when there's something else which is supposed to
> keep
> >> it on?
> >>
> >> Same for patch 8.
> >
> > pm_runtime_get_sync only returns -EACCES when runtime pm has been
> > disabled in which case the GPU is on and pm_runtime_put_autosuspend
> > won't do anything.
> 
> It decreases dev->power.usage_count, and my worry was that
> pm_runtime_get_sync might not have increased it when it returns an
> error. But it turns out that it does increase it unconditionally.
> 
> 
> However, I'm afraid there seems to be another related problem: If
> amdgpufb_open returns an error, amdgpufb_release won't be called
> AFAICT.
> In which case amdgpufb_open needs to call pm_runtime_put_autosuspend
> in
> the error case, or the GPU can never power off after that.

Good point.  I'll fix that up.

Alex

> 
> 
> --
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer