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

[Michel Dänzer]

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.


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