[PATCH 3/4] drm/radeon: consolidate uvd/vce initialization, resume and suspend.

[Jerome Glisse]

On Wed, Mar 16, 2016 at 2:03 PM, Christian König
<deathsimple at vodafone.de> wrote:
> Am 16.03.2016 um 13:48 schrieb Jérôme Glisse:
>>
>> From: Jérome Glisse <jglisse at redhat.com>
>>
>> This consolidate uvd/vce into a common shape for all generation. It
>> also leverage the rdev->has_uvd flags to know what it is useless to
>> try to resume/suspend uvd/vce block.
>>
>> There is no functional changes when there is no error. On error the
>> device driver will behave differently than before after this patch.
>> It should now safely ignore uvd/vce errors and keeps normal operation
>> of others engine. This is an improvement over current situation where
>> we have different behavior depending on GPU generation and on what
>> fails.
>>
>> Finaly this is a preparatory step for a patch which allow to disable
>> uvd/vce as a driver option.
>>
>> This have only been tested on southern island so please test it on
>> other generations (i do not have hardware handy for now).
>>
>> Signed-off-by: Jérôme Glisse <jglisse at redhat.com>
>> Cc: Alex Deucher <alexander.deucher at amd.com>
>> Cc: Christian König <christian.koenig at amd.com>
>
>
> NAK, skipping UVD and VCE suspend/resume when initialization fails should
> already be implemented.
>
> There might be some (quite some) bugs in there, but that doesn't justify
> reworking the initialization over all different generations. Especially
> since you don't have hardware to test all of them.
>
> Just make sure that radeon_uvd/vce_fini() is called when something goes
> wrong and/or that the UVD/VCE BO is properly released.
>
> Regards,
> Christian.

Current code is a mess when it comes to handling error related to
uvd/vce. This patch consolidate control flow in something easy to
follow. You can check that there is absolulety no control flow change
for the case where uvd/vce works and thus it does not break anything
that works. It will only gracefully fails and cleanup if things go
wrong. So while i have not tested on other hw i am confident that this
does not introduce regression.

I tried to do it without consolidation but it ended up in adding even
more if() levels that line did begins after 80colums. So please
reconsider because this is an improvement over existing code.

Jérôme