On Wed, Mar 16, 2016 at 2:03 PM, Christian König <deathsimple@xxxxxxxxxxx> wrote: > Am 16.03.2016 um 13:48 schrieb Jérôme Glisse: >> >> From: Jérome Glisse <jglisse@xxxxxxxxxx> >> >> 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@xxxxxxxxxx> >> Cc: Alex Deucher <alexander.deucher@xxxxxxx> >> Cc: Christian König <christian.koenig@xxxxxxx> > > > 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
