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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 16.03.2016 um 15:59 schrieb Jerome Glisse:
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.

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

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux