Am 16.03.2016 um 15:59 schrieb Jerome Glisse:
On Wed, Mar 16, 2016 at 2:03 PM, Christian König deathsimple@vodafone.de wrote:
Am 16.03.2016 um 13:48 schrieb Jérôme Glisse:
From: Jérome Glisse jglisse@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@redhat.com Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@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.
Well then please point out at the example of the SI or CIK code what exactly is missing here.
Please also note that VCE/UVD has dependencies on power management, so that when they are once initialized they should NOT be turned off again.
I only briefly skimmed over your patch, but it actually looks like to me that you broken that by trying to cleanup the initialization routine.
Regards, Christian.
Jérôme